refactor(basket-compare): rework into a tested, low-noise basket diff#1284
Merged
Conversation
Importing a regenerated Director basket overwrites every object it
contains, silently clobbering manual adjustments. basket-compare exists
to surface those first, but the previous version only printed changed
objects and dropped additions and removals (its add/remove detection was
broken by a leaked loop variable), so it missed cases such as a datafield
an import would delete.
- report objects only in the left basket, only in the right, and changed
in both; default shows changed only, --all adds the "Only in ..." tables
- replace the ten per-class copy-paste compare_/get_diff functions and the
deepdiff dependency with one generic engine over a normalised
{uuid: {field: value}} shape
- collapse a whole added/removed sub-object (service, command argument,
field binding) into a single row instead of one row per attribute
- name field bindings by datafield varname plus a truncated,
dash-preserving uuid; diff list-valued fields element by element
- suppress known Director export noise: the is_required flag and the
number-vs-string serialisation of the same value
- add golden-file unit tests under tools/unit-test/basket-compare/
- update TOOLS.md
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.
What
Reworks
tools/basket-comparefrom a deepdiff-based, change-only script into asingle generic diff engine, and adds golden-file unit tests.
Why
When a new plugin release ships a regenerated Director basket, importing it
overwrites every object it contains - silently clobbering any manual adjustment
an admin made.
basket-compareexists to surface those before the import.The previous version only printed changed objects and silently dropped
additions and removals (and its add/remove detection was broken by a leaked
loop variable). As a result it missed real cases - e.g. a datafield that an
upcoming import would delete never showed up.
Changes
and changed in both. By default only changed objects are shown (an import
never touches an object it does not contain);
--alladds theOnly in ...tables.
compare_*/get_*_difffunctions and thedeepdiffdependency with one generic engineover a normalised
{uuid: {field: value}}shape.binding) collapses into a single row instead of one row per attribute;
dash-preserving uuid (still greppable against the basket);
is_requiredflag (which doesnot round-trip) and values stored as a number on one side and the same
digits as a string on the other.
tools/unit-test/basket-compare/golden-file tests (fixtures +run), robust to the trailing-whitespace / end-of-file pre-commit hooks.TOOLS.mdbasket-comparesection updated.Impact
On a real upgrade export the output dropped from ~1700 to ~440 lines while now
catching object additions/removals the old version missed.
Notes
tools/scripts are not auto-discovered by the CI test harness(
run-unit-testscoverscheck-plugins/only); the new test runs viatools/unit-test/basket-compare/run. Wiring tools tests into CI is a possiblefollow-up.
basket-compareis an internal maintainer tool, not partof the shipped, admin-facing plugin collection.