test(e2e): assert expected event and process counts per fixture#70
Open
izzet wants to merge 1 commit into
Open
test(e2e): assert expected event and process counts per fixture#70izzet wants to merge 1 commit into
izzet wants to merge 1 commit into
Conversation
The e2e assertions were all structural -- view counts, layer counts, a checkpoint glob, client shutdown -- and every one of them holds on an empty result. A fixture that silently loads zero events kept CI green while testing nothing. That is not hypothetical. Before llnl#63 the dftracer fixtures shipped uncompressed .pfw traces the indexer cannot read, so dftracer-posix and dftracer-dlio-prev both analysed 0 events and the suite passed. Pin the ground truth per fixture, measured with a single worker: dftracer-dlio 125,669 events / 1 process dftracer-dlio-prev 18,039 events / 8 processes dftracer-posix 2,056 events / 1 process Fixtures absent from the table are skipped, so this does not constrain the darshan and recorder cases.
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
1 participant
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.
Why
Every assertion in
_test_e2eis structural — view counts, layer counts, a checkpoint glob, client shutdown — and all of them hold on an empty result. A fixture that silently loads zero events keeps CI green while testing nothing.That already happened. Before #63 the dftracer fixtures shipped uncompressed
.pfwtraces, which the dftracer-utils indexer cannot read, sodftracer-posixanddftracer-dlio-prevboth analysed 0 events and the suite passed. #63 fixed the fixtures but nothing stops it recurring.It is also live risk right now: #66 as currently pushed carries the pre-#63 (uncompressed) fixture forward under a new name, and #69 makes a multi-worker read silently drop events. Both are the same failure shape — a short or empty read that no test notices.
What
A per-fixture ground-truth table, asserted after
analyze_trace():dftracer-dliodftracer-dlio-prevdftracer-posixdftracer-posixat 2,056 matches the figure in #63's commit message. Counts were measured with a single worker, which is what the e2eLocalCluster(processes=False, ...)gives, so they are not exposed to #69.Fixtures absent from the table are skipped, so darshan and recorder are unconstrained — this does not collide with #67 dropping them.
Verification
pytest tests/test_e2e.py -m full -k dftracer— 6 passedNote for #66
#66 touches
tests/test_e2e.pytoo and will need a trivial rebase. When the fixtures are renamed, the table keys change with them — and the assertion is what would have caught the stale-fixture regression that PR currently carries.