Skip to content

Fix false-positive in EnrichmentPipelineTest by introducing mock Config with DummyModule#151

Merged
jnioche merged 1 commit intoDigitalPebble:mainfrom
davide954:test/pipeline-unit-coverage
Feb 21, 2026
Merged

Fix false-positive in EnrichmentPipelineTest by introducing mock Config with DummyModule#151
jnioche merged 1 commit intoDigitalPebble:mainfrom
davide954:test/pipeline-unit-coverage

Conversation

@davide954
Copy link
Contributor

@davide954 davide954 commented Feb 16, 2026

Problem

The existing EnrichmentPipelineTest used new Config() directly,
which returns an empty module list in the test environment.

As a result, the enrichment verification loop never executed, making
the test pass silently regardless of whether the pipeline logic was
correct — a false positive flagged by @tejas-2232.

Solution

Following the approach agreed with @jnioche discussed in #142 (mock Config
rather than expanding the schema with all production columns):

  • DummyModule: a minimal EnrichmentModule that declares a
    single dummy_enriched column and sets it to 42.0 in process().
  • createMockConfig(): builds a Config pre-loaded with the
    DummyModule and injects the matching config entry via reflection,
    following the same pattern used in ConfigTest.
  • assertFalse(modules.isEmpty()): guard assertion that fails
    fast if modules are not loaded, preventing silent false positives.
  • Dynamic indices: all field lookups use schema.fieldIndex(...)
    instead of hardcoded positions, making the test robust against
    schema changes.

What this does NOT change

  • No changes to production code.
  • No new test dependencies (no Mockito).
  • The test scope remains focused on the Usage-filter logic of
    EnrichmentPipeline (Usage → enriched, Tax/Fee → skipped).

Verification

The parameterized test now actually exercises the enrichment path:

  • Usagedummy_enriched is populated (42.0)
  • Taxdummy_enriched remains null
  • Feedummy_enriched remains null

…Test

The test was silently passing (false positive) because
ew Config()
returns an empty module list in the unit test environment, causing the
enrichment verification loop to never execute.
@davide954
Copy link
Contributor Author

davide954 commented Feb 18, 2026

@jnioche I watch out your PR #149 and probably this tests will not succeed correctly with your new implementations, so I won't do anything till you merge that PR, I understand the sensitivity of this code change. I'll keep you posted.

@jnioche
Copy link
Member

jnioche commented Feb 18, 2026

Hi @davide954, am away this week. Will comment and review when I'm back

@jnioche
Copy link
Member

jnioche commented Feb 21, 2026

@jnioche I watch out your PR #149 and probably this tests will not succeed correctly with your new implementations, so I won't do anything till you merge that PR, I understand the sensitivity of this code change. I'll keep you posted.

I merged that PR. It has no impact on your PR

@jnioche jnioche added this to the 0.9 milestone Feb 21, 2026
@jnioche jnioche merged commit 61ba98d into DigitalPebble:main Feb 21, 2026
2 checks passed
@jnioche
Copy link
Member

jnioche commented Feb 21, 2026

thanks @davide954

@jnioche
Copy link
Member

jnioche commented Feb 21, 2026

@jnioche I watch out your PR #149 and probably this tests will not succeed correctly with your new implementations, so I won't do anything till you merge that PR, I understand the sensitivity of this code change. I'll keep you posted.

I merged that PR. It has no impact on your PR

actually it does, sorry! Just fixed it

@davide954
Copy link
Contributor Author

ok now I see it works, thanks @jnioche.

@davide954 davide954 deleted the test/pipeline-unit-coverage branch February 21, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants