fix duplicate spec ids for large test suites with copy/paste structure #82
Conversation
mdmosby
changed the title
| return cls._repo_root[path] | ||
| except KeyError: | ||
| pass | ||
| def _compute_repo_root(cls, path: Path) -> Path: |
There was a problem hiding this comment.
It seems like this method would return user paths and would result in unstable spec IDs for different users. Should we actually include this in the ID hash, or only use it to compute the relative paths? Thoughts @tjfulle ?
There was a problem hiding this comment.
As a counter argument, the returned path is simply the parent if we don't find a VC directory in the tree. Generally speaking, constructing stable hashes from such a path would be very difficult or impossible so maybe this fall-back case should instead return /? This would make rel_repo be the full-path to the file and we can drop repo_root from the hash.
|
Thanks @mdmosby - putting the repo root and not the relative path was bug, thanks for the fix! |
2 participants
The updated (much faster) spec ID computation did not include quite enough information to ensure unique IDs are generated for large test suites. For example, if generator files had the same hash, root, and parameter set, they would generate the same ID. For SIERRA's large test suite, this structure is fairly common (with additional/different assets influencing test behavior), and including the generator path relative to repo-root is sufficient to differentiate specs.
This PR includes the additional information and now successfully generates specs for the full SIERRA test suite without duplicates.