Repository navigation
fix(test): compare canonical snapshot paths - #3339
Conversation
Compare real directory identities in the sync fixture. Preserve snapshot marker and directory sync assertions. Handle macOS temporary directory aliases.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3339 +/- ##
============================================
- Coverage 54.29% 52.84% -1.45%
+ Complexity 2616 3 -2613
============================================
Files 770 503 -267
Lines 67952 48402 -19550
Branches 9213 6963 -2250
============================================
- Hits 36892 25580 -11312
+ Misses 27482 20196 -7286
+ Partials 3578 2626 -952 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The one-line change makes checkMarkerDirectorySync compare the real paths of the two directories instead of their spellings, which is the right fix for the macOS /var versus /private/var alias, and it does not weaken the check on Linux. I found no issues. Score 9/10. Evidence: read the full exact-head diff at ef80026 (1 file, +1/-1) and the surrounding test and production code. RocksDBSnapshotRestore builds its marker from new File(data).getCanonicalFile() (RocksDBSnapshotRestore.java:106, :311-312) and passes this.marker.getParent() to forceDirectory at :438, :526 and :571, so production always hands over the canonical parent, while the test derives marker from the TemporaryFolder path, which is not canonical on macOS. The compared directory is the TemporaryFolder root created at the top of the helper and it exists at all three calls, so toRealPath() cannot throw NoSuchFileException there. The other path equalities in the class (:1419, :1441) already use toRealPath(), and I found no other lexical comparison against a canonicalized production path. The class is part of UnitTestSuite (UnitTestSuite.java:257), and the server-rocksdb / server_rocksdb (Java 17) job passed at this head. I did not run the test on macOS myself. codecov/project reports a failure, which comes from the scoped CI run uploading fewer modules than the base (503 files against 770) and not from this change.
Purpose of the PR
Snapshot directory-sync tests fail on macOS when the same temporary directory is reported as both
/var/...and/private/var/....Main Changes
Compare canonical directory identities instead of lexical path spellings. The marker and directory-sync assertions remain in place; production snapshot behavior is unchanged.
Verifying these changes
RocksDBSnapshotRestoreTestreproduces the alias failure before the change and passes afterward.Does this PR potentially affect the following parts?
Documentation Status
Doc - TODODoc - DoneDoc - No Need: test fixture only.Documentation files in this PR or paired hugegraph-doc PR:
Conceptual overview
Conceptual test-only fix: resolve both directory paths before comparison; existing assertions and production behavior remain unchanged.
Conceptual illustration; not test or release-acceptance evidence.