Fix the path cache test fakes and cover the listing sizes they missed - #309
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
test/test_path_cache.pyfails onmain— 9 of its 10 tests:#307 added
listdir_info,_cache_listing_sizesandself.listing_sizestoGFALFileInterface, readingentry.is_dirandentry.sizeoff a listing. The test file, addedin #304, models a listing entry as
namedtuple("Entry", ["name"])and builds its fake interfaceby assigning
voms_token/path_cache/verboseby hand instead of calling the real__init__— so neither the new attribute nor the new fields existed.Confirmed to predate the current
main: the suite already fails at59daf8d(#307), before#306 merged. The production code is correct; only the test doubles were stale. Nothing runs this
suite in CI, which is why it went unnoticed.
Solution
FakeGFALFileInterfacenow calls the real__init__with only the grid-proxy lookupstubbed out, rather than mirroring the attributes it sets. That is what makes the fake track
GFALFileInterfacefrom now on: hand-mirroring is precisely what broke whenlisting_sizesappeared, and it broke silently until something read it.
Entrycarries the fields ofRunKit.grid_tools.FileInfothatlaw_gfalactually reads —name,size,is_dir— with defaults so the existing cases stay readable.{name: size}as well as to a list of names, and anentry that is itself a key of the tree is reported as a directory, as a real listing would.
Also: the listing sizes had no test
listdir_infois the reason the fakes broke and had no coverage of its own. Three tests added:exists()that already listed the directory costs no secondgfal-ls— the actual claim in its docstring;{}rather than raising.Testing
test/test_path_cache.py: 13 passed (was 1 passed / 9 failed).test/directory withANALYSIS_PATHset: 79 passed.not entry.is_dirfilter in_cache_listing_sizes→ the exclusion test fails;_cache_listing_sizescall inlistdir→ the reuse test fails.law_gfal.pywas restored unmodified afterwards.No docs change: internal only — test doubles and test coverage, no user-visible behaviour.