Refactor Scuro tests - #2607
Open
shieru1214 wants to merge 5 commits into
Open
Conversation
Part of identifying redundant and overly component-level tests under
tests/scuro and refactoring them into smaller unit-level tests. The join
tests are the case where the refactoring uncovered defects in the code
under test, so the fix and the tests that prove it are kept together.
test_multimodal_join (5 -> 11). The five tests ran the full
join -> ResNet -> window aggregation -> combine chain but only asserted
"not None" and "len == N". Mutation testing showed how little they
covered: zeroing out the entire join result left all five passing, and
the data they asserted on was 100% NaN because MelSpectrogram emits a
single row per instance. They are replaced by TestJoinMapping - six unit
tests that build two modalities with hand-written timestamps and call
execute() directly, using a small deterministic SpyRepresentation instead
of ResNet - plus a thinner integration layer. The four chunk variants
become one property test asserting that chunked and unchunked joins agree
element-wise, and one end-to-end ResNet smoke test is kept deliberately.
joined.py carries three defects, all still present on main:
- the last right-hand sample of every instance was dropped, in both the
"<" and the equality branch (off-by-one in the loop condition)
- the no-match fallback averaged an empty slice whenever c was 0, which
is what produced the all-NaN join output above
- the equality branch called .append() on an ndarray and had therefore
never been executed; it now collects matches and concatenates
Reverting joined.py to its state on main while keeping this commit's
tests fails 10 of the 11.
Part of identifying redundant and overly component-level tests under
tests/scuro and refactoring them into smaller unit-level tests. These
three files change tests only; no source behaviour is affected.
test_fusion_orders (4 -> 1). Four methods that differed only in the fusion
operator and three booleans become one table plus a subTest loop. The
commutativity column is read from the operator's own "commutative"
attribute rather than duplicated in the table, so an operator whose
declaration contradicts its implementation now fails here. The original
concat test compared the wrong pair, so "a pairwise chain equals the
n-ary form" had never been asserted for Concatenation; it is True.
test_text_context_operators. Randomly generated sentences only allow
invariant assertions ("a chunk has at most max_words"), which pass for a
large family of wrong implementations. With fixed input the expected
chunks and character spans can be written down, and the span-returning
operators are cross-checked against the string-returning ones. setUpClass
becomes setUp because the *Indices operators write text_spans into the
shared metadata.
test_scheduler. test_deadlock_when_no_nodes_are_runnable asserted
is_finished() after a "while not is_finished()" loop - a tautology. What
separates it from test_finished_when_no_nodes_are_runnable is
scheduler.success, which was never checked; with the old assertion the
test passed even when the memory budget was raised so that it was no
longer a deadlock. test_get_ready_nodes_second_level had no guard on the
list it looped over, so it passed vacuously if the scheduler returned
nothing.
test_window_operations.py contained three groups of tests whose bodies
were identical apart from a single argument:
- test_static_window and test_dynamic_window differed only in the
operator name. Both assert the same contract: exactly num_windows
segments per instance, whatever the input length.
- the audio, video and text window aggregation tests each called the
same helper with a different ModalityType. create1DModality returns
the same random matrix for all three types, only the metadata label
differs, and window_aggregation dispatches on the data layout rather
than the modality type, so the three ran identical code over
identical numbers.
- the 3d and 2d output shape tests differed in the shape tuple.
Window aggregation compresses the first (time) axis and leaves the
feature axes untouched, so (num_windows,) + dims[1:] covers both.
Each group becomes one subTest-parameterised test: 7 test methods become
3, while all 20 combinations still run and are still reported
individually, now labelled with the modality, aggregation, shape and
operator that failed. The bare asserts in the merged bodies become
assertEqual so a failure reports the values instead of just the line.
No test coverage is removed and no source behaviour changes.
Three groups of tests differed only in which modality they built, while
every assertion lived in a helper they all shared:
- test_unimodal_optimizer had five tests calling
optimize_unimodal_representation_for_modality with zero assertions of
their own. The helper already loops over the modality list, so the
multi-modality case is a set with two entries rather than a separate
shape.
- test_hp_tuner had two tests calling run_hp_for_modality, again with
every assertion in the helper. A structural comparison does not find
these: the two build their data with different generator functions
taking different numbers of arguments.
Each group becomes one subTest-parameterised test over a table of
modality sets, with the construction moved into a factory. Test methods
go from 6 to 2 across the two files; every case still runs and is now
reported with the modality that failed.
The factories keep the inputs exactly as the individual tests had them,
including the two places where they differed by accident: video used ten
frames where image used one, and the multi-modality case built its text
with the generator default of one sentence rather than the ten the
standalone text case used.
test_unimodal_representations.test_audio_representations built its
modality by repeating the body of _create_audio_modality, down to the
same signal length, and now calls that helper instead.
No test coverage is removed and no source behaviour changes.
test_data_loaders had three tests -- audio, video and image -- whose bodies were identical apart from the loader class and the number of dimensions the loaded arrays are expected to have. Loading is the same contract for every loader: one array plus one metadata entry per instance, at the dimensionality that modality has. That is now one subTest-parameterised test over a table of (loader, modality, ndim). The stats tests stay as they are. Each stats class exposes a different set of fields -- audio has a sampling rate and an average length, video adds a frame count to the image dimensions -- so beyond the instance count there is no shared assertion to parameterise, and folding them together would hide the differences rather than surface them. The text loader test also stays separate: it asserts the loaded instances are strings rather than arrays of a given rank. No test coverage is removed and no source behaviour changes.
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.
Overview
The existing Scuro tests contained several groups of methods that exercised the same code with different modalities or operators. The join tests had a separate problem: they ran a long processing chain but did not check the temporal mapping produced by the join itself.
This PR parameterizes the repeated cases, separates join mapping from integration coverage, strengthens several weak assertions, and fixes three defects exposed by the new join tests.
This work is based on
24fadd7149(2026-09-01).test_data_loaders.py8 -> 6methodstest_multimodal_join.py5 -> 11testsmodality/joined.pytest_unimodal_optimizer.py13 -> 9methodstest_hp_tuner.py3 -> 2methodstest_unimodal_representations.pytest_window_operations.py27 -> 23methodstest_text_context_operators.pytest_fusion_orders.pytest_scheduler.pyAcross the nine test files, the number of test methods changes from
95to87, while the number of subtests increases from224to264. The merged inputs are still executed separately, and a failure reports the relevant loader, modality, operator, or shape.The repeated groups were located by scanning all 19
test_*.pyfiles undertests/scurorather than only the ones that looked duplicated. After this PR the same scans report no remaining groups. The two passes are described under Validation.Runtime
Per-file runtimes compare this branch with the baseline commit. Both revisions were measured using the same environment and command:
(100, 8)/(100, 8, 8)40 x 100to4 x 8; scheduler fixtures are unchangedThese are local single-run measurements, so sub-second differences are within normal variation. The overall reduction mainly comes from the join tests, where real ResNet execution was reduced from five tests to one. Absolute runtimes will vary by machine.
Changes
Loading
The audio, video, and image loading tests in
test_data_loaders.pyhad the same body. Only the loader, modality type, and expected array rank differed.The stats tests remain separate because each one checks different metadata fields. The text loader also remains separate because its result is a list of strings rather than an array.
Join
Why split the tests?
The five original methods in
test_multimodal_join.pyall ran a variation of the same complete workflow:This mixed two different questions in every test:
The generated inputs made the exact join result difficult to state, so the old tests could only check that later pipeline stages returned a value. They could therefore pass even if the join selected the wrong rows. Running ResNet in every
case also made the file slow without adding five distinct checks of ResNet.
The two responsibilities now live in separate classes:
TestJoinMappingdirectly constructs the smallest modalities needed byJoinedModality.execute(). It does not use data loaders, representation operators, torch, or librosa. Its six tests check:execute(starting_idx=2)uses the matching right instancesTestMultimodalJoinkeeps the workflow-level checks. A deterministicSpyRepresentationreplaces ResNet where the test only needs a representation that transforms both sides of the join. The four chunk configurations are nowcompared against the unchunked result, rather than only checked for completion. One test still uses the real ResNet path as a smoke test.
This separation gives the two layers different failure meanings: a mapping-test failure points to timestamp assignment, while an integration-test failure points to representation, chunking, or pipeline composition.
The exact mapping assertions exposed three defects in
joined.py:len(idx_2) - 1boundary.append()on an ndarrayRepresentation Operators and optimization
The optimizer tests remain integration tests because they still build and run the representation DAGs. The change here is focused on removing wrapper methods that supplied different inputs to the same workflow and assertions.
In
test_unimodal_optimizer.py, these five methods:all ended by calling
optimize_unimodal_representation_for_modality(). They are now represented by one data table and one modality factory:The original input details are preserved, including the ten-frame video case and the one-sentence text input used by the mixed text-and-image case. This keeps the same optimizer paths while putting their shared setup and assertion in
one place.
test_hp_tuner.pyhad the same pattern on a smaller scale:Both cases still run the same optimizer and hyperparameter-tuning workflow. The change removes duplicated modality construction; it does not replace the end-to-end HPO coverage with a mock.
test_unimodal_representations.pyonly removes duplicated audio setup by using its existing_create_audio_modality(signal_length=200)helper. Thetransform()andapply_representation()tests remain separate because they exercise different execution paths.Window and text context
The first seven methods in
test_window_operations.pyformed three duplicate groups. Each group is now one parameterized test, but the reason for combining it is different:test_static_window,test_dynamic_windowtest_fixed_window_count_operatorswith 2 subteststest_window_aggregation_on_1d_modalitieswith 12 subtestsDataLayout, not the modality labeltest_window_aggregation_on_nd_modalitywith 6 subtests(num_windows,) + dims[1:]The resulting case matrix is:
All 20 original combinations still run. The modality, aggregation, dimensions, and operator are subtest labels, so a failure identifies the exact combination. This reduces seven wrappers to three tests without treating genuinely different window behavior elsewhere in the file as interchangeable.
test_text_context_operators.pykeeps its four separate methods because sentence boundary and overlap operators, and their string and span outputs, are distinct paths. The input is changed from random text to two fixed sentences so the tests can assert complete chunks and exact character spans. The span-returning results are also checked against the corresponding slices of the original strings.Fusion and scheduling
The four methods in
test_fusion_orders.pyrepeated the same sequence of binary and n-ary fusion calls for Average, Concatenation, RowMax, and Hadamard. They are now one property table:In
test_scheduler.py, the deadlock test now checks that scheduling failed and that no nodes completed, instead of checking a state that was true for both success and failure. The second-level scheduling test also verifies that exactlyone node was returned before iterating over the result.
Validation
Duplicate review
All 19
test_*.pyfiles intests/scurowere checked for two kinds of repetition:The shared-helper check found the
test_unimodal_optimizerandtest_hp_tunergroups that the structural comparison missed because their input construction was different. After the refactoring, no remaining groups match either rule.Mutation checks
Each merged group was mutation-tested to confirm that the affected subtests fail while the sibling cases in the same parameterized test still pass. Other tests outside the merged group may also fail when they exercise the mutated code; those expected failures are not listed below.
The table shows representative checks from the full set:
StaticWindowreturn one segment feweroperator='StaticWindow'casesDynamicWindowandWindowAggregationcases_sum_aggreturn the meanaggregation='sum'casesmean,max, andminLIGHTWEIGHT_REGISTRY[VIDEO]modalities='video'text,image,audio, andtext+imageLIGHTWEIGHT_REGISTRY[TEXT]modalities='text'andmodalities='text+image'image,audio, andvideoVideoLoader.load()raiseloader='VideoLoader'AudioLoaderandImageLoaderThe
TEXTmutation also confirms that the combinedtext+imagecase constructs and exercises both modalities.Join regression checks
To check that the new join tests cover the three fixes, only
joined.pywas restored to its state at24fadd7149while the new tests were kept. Pytest then reported:The failures include the exact mapping, no-match fallback, equality join, chunk offset, and chunk-consistency checks.
Suite state
python -m pytest tests/scuro -qpython -m unittest discover -s tests/scuro -p 'test_*.py'python -m black --check tests/scuro/systemds/scuro60% -> 60%systemds/scuro/modality70% -> 72%modality/joined.py70% -> 76%Shortcomings
The runtime reduction is concentrated in one file. About 49 of the 55 seconds saved come from
test_multimodal_join.py, where real ResNet execution was reduced from five tests to one. The changes in the other eight files mainly improve test structure and assertions; their runtime differences are within normal measurement variation.The refactoring does not turn every affected test into a unit test. Only the six
TestJoinMappingtests isolate a single component.test_unimodal_optimizerandtest_hp_tunerstill build and run complete representation DAGs. Replacing those workflows with fakes would make them smaller and faster, but would also remove their existing end-to-end coverage, so that change is not made here.