Refactor known_diffs.py into module - #3179
Open
Jongmassey wants to merge 5 commits into
Open
Conversation
Jongmassey
force-pushed
the
Jongmassey/known_diffs_refactor
branch
from
July 31, 2026 13:23
864b19e to
c4203ca
Compare
This refactor hides all the const dicts and types only used within the known_diffs module from the outside world. The only place the inner details of this module are tested is in test_known_diffs.py All other tests that previously monkeypatched the internal data of something in known diffs now monkeypatch the functions that access that data. This ensures that the unit tests for these access functions test the access function, and that the tests of things that call them are testing that the calling function handles their returns correctly. For example, the tests in test_version.py now test that the returned data regarding icd-10 differences is added to the view context correctly, rather than testing that the various known difference access functions perform correctly (as this is the responsibility of those functions own unit tests). An exception to the above is release_builder and its tests which are tightly coupled to the RubricDifference dataclass.
Jongmassey
force-pushed
the
Jongmassey/known_diffs_refactor
branch
from
July 31, 2026 13:24
c4203ca to
a443f18
Compare
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.
These commits refactor the large, all-encompassing known_diffs.py into a module.
Each sort of difference (as defined by having its own const dict) is moved into its own file and has its helper functions with it.
Where const dicts were accessed outside of the module these are moved to a helper function for consistency.
The other major change is to tests which test functions that make use of the known difference data in some way, via a pre-existing helper function. These have been changed to monkeypatch the helper function rather than the underlying data. There were existing unit tests for the helper functions which patched the underlying data within test_known_diffs.py.
This clarifies the responsibilities of these tests - the known diff helper function unit tests test that the helper function behaves correctly, the unit tests for the things that use the output of these helper functions (e.g. views) test that these call the helper function correctly and consume their outputs correctly - i.e. that the code within the function under test is correct, not testing its dependencies which are tested elsewhere.
Overall this improves readability, clarifies responsibilities, and hides internal implementation details from the rest of the codebase.
There remains some coupling between release_builder.py (and its tests) and the
RubricDifferencedataclass. This was too tricky to unpick at this moment.