Repository navigation
Stop the overalloc oracle test counting Project's UID-0 resource (#518) - #520
Merged
Merged
Conversation
`project_assignment_and_resource_offsets` failed on every snapshot of the real .mpp corpus, not just the one it named. Two problems, the second hidden behind the first. The decoder is right in both; only the test changes. 1. Project's XML always lists an implicit unassigned resource, UID 0, and read_mspdi keeps it. Since #507 the decoder reads the true resource UID (+4, as the module doc says; it read the row ID at +0 before) and skips UID 0, which nothing looks up: unassigned work carries the binary sentinel -65535 and is skipped before any resource lookup. So the count was off by exactly one on all 47 pairs — 34-resource-work decodes [1] against an oracle of [0, 1]. The test now compares only real resources, and pins both halves: the XML still lists UID 0, and the decoder still leaves it out. Checked by reintroducing UID 0 in the decoder: the new assertion fires on the first snapshot. 2. `checked >= 50` could never pass on the corpus anyone can fetch. yeroo/mpp-corpus provides only snapshots/, 47 eligible pairs. The other folders the test walks are generated locally with Project and pywin32, and neither the Mac nor the Windows machine has them. The guard now counts snapshot pairs and requires a complete fetch's 47, so a partial fetch still fails loudly while generated folders are checked on top. Checked by raising the bound to 48: it fails naming 47. The paths are now sorted before checking. read_dir order is whatever the filesystem keeps: macOS reached 31-calendar-hours first, Windows another file, and a failure common to every snapshot looked peculiar to 31. Measured on macOS with the fetched corpus (yeroo/mpp-corpus b02a8e0): `cargo test -p mppread --release --lib` goes from 1 failure to 83 passed. With `--no-fail-fast -- --include-ignored` the only failures left are the two #517 tests on 44-baseline, unchanged. With no corpus present, as in CI, the test still returns early and everything passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018qSpykxfaLiQSDxjsqbQUm
Owner
Author
|
Windows (corpus b02a8e0) on a44389d: |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
1 task
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.
Closes #518. Test-only change. The decoder is right; the corpus test was comparing against the wrong thing.
overalloc::tests::project_assignment_and_resource_offsetsfailed on every snapshot of the real.mppcorpus, not just the one it named. Two problems, the second hidden behind the first.1. Project's implicit UID-0 resource
Project's XML export always lists an implicit unassigned resource with UID 0, and
read_mspdikeeps it. Since #507 the decoder reads the true resource UID (+4, as the module doc says; before, it read the row ID at +0) and skips UID 0. That skip is right: the only caller, the OverAllocated derivation, never looks it up, because unassigned work carries the binary sentinel-65535and is skipped before any resource lookup.So the count was off by exactly one on all 47 pairs:
Every real resource matched. The test now compares only real resources and pins both halves of that design, so neither side can drift unnoticed:
2.
checked >= 50could never pass on the fetchable corpusyeroo/mpp-corpusprovides onlysnapshots/, which is 47 eligible pairs. The other folders the test walks (task-fields/,order/,manual/,lag/,progress/) are generated locally with Project and pywin32 (corpus/tools/gen_mpp_task_field_cases.py), and neither the Mac nor the Windows machine has them. The guard now counts snapshot pairs and requires a complete fetch's 47. A partial fetch still fails loudly, and generated folders are checked on top when present.3. Sorted paths
read_dirorder is whatever the filesystem keeps. macOS reached31-calendar-hoursfirst and Windows another file, so a failure common to every snapshot looked peculiar to 31, and #518 was first filed under that name. The paths are sorted now. No assertion changes.Proof, on macOS with the fetched corpus (
yeroo/mpp-corpusb02a8e0)cargo test -p mppread --release --lib... --no-fail-fast -- --include-ignored44-baseline, unchangedThe new assertions were checked against their failure modes. Each was broken on purpose and then reverted:
cargo fmt --checkis clean, andcargo clippy -p mppread --all-targets -- -D warningsexits 0.One deviation from the agreed plan, worth stating. The plan said to also assert "every eligible pair was checked". Every mismatch in this test panics, and every skip is an explicit
continuebefore a pair is eligible, so that assertion would be vacuous. I left it out, and the snapshot minimum is the guard.#517 (the
44-baselinebaseline-work failure) is deliberately untouched.🤖 Generated with Claude Code
https://claude.ai/code/session_018qSpykxfaLiQSDxjsqbQUm