Skip to content

Fixes causal test results misalignment when skip:true tests are present#378

Merged
f-allian merged 7 commits intomainfrom
f-allian/chore
Feb 5, 2026
Merged

Fixes causal test results misalignment when skip:true tests are present#378
f-allian merged 7 commits intomainfrom
f-allian/chore

Conversation

@f-allian
Copy link
Collaborator

@f-allian f-allian commented Jan 30, 2026

  • In causal_testing/main.py, the save_results method now filters out tests marked with "skip": true in the test configuration, so only active tests are paired with their results and saved.

  • In tests/main_tests/test_main.py, both test_ctf and test_ctf_exception_silent now use only active tests (not skipped) when checking which tests passed, aligning the test logic with the updated main code.

  • Closes issue Misaligned test results when skip:true tests are present in causal tests #377

@f-allian f-allian self-assigned this Jan 30, 2026
@f-allian f-allian added the bug Something isn't working label Jan 30, 2026
@github-actions
Copy link

github-actions bot commented Jan 30, 2026

🦙 MegaLinter status: ⚠️ WARNING

Descriptor Linter Files Fixed Errors Elapsed time
⚠️ PYTHON black 32 1 1.2s
✅ PYTHON pylint 32 0 6.06s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

@codecov
Copy link

codecov bot commented Jan 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.87%. Comparing base (2fac254) to head (e262f67).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #378      +/-   ##
==========================================
+ Coverage   97.29%   97.87%   +0.57%     
==========================================
  Files          27       27              
  Lines        1555     1551       -4     
==========================================
+ Hits         1513     1518       +5     
+ Misses         42       33       -9     
Files with missing lines Coverage Δ
causal_testing/main.py 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd9241f...e262f67. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@f-allian f-allian marked this pull request as ready for review January 30, 2026 13:02
@f-allian f-allian requested a review from jmafoster1 January 30, 2026 13:02
Copy link
Collaborator

@jmafoster1 jmafoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution means that skipped tests don't make it into the results file. Personally, I'd prefer they were still there so we have a record of all the test outcomes so we know if any where skipped (as opposed to not generated/considered). It'd be good to preserve order too if we can, although putting skipped test cases at the end is more acceptable than not having them there at all.

@f-allian
Copy link
Collaborator Author

f-allian commented Feb 2, 2026

@jmafoster1

Is this what you're after? For example, a causal test case with skip:true:

    {
      "name": "treatment --> outcome",
      "estimator": "LinearRegressionEstimator",
      "estimate_type": "coefficient",
      "effect": "direct",
      "treatment_variable": "treatment",
      "formula": "treatment ~ outcome",
      "alpha": 0.05,
      "skip": true,
      "expected_effect": {
        "treatment": "SomeEffect"
      }
}

now has a corresponding result:

  {
    "name": "treatment --> outcome",
    "estimate_type": "coefficient",
    "effect": "direct",
    "treatment_variable": "treatment",
    "expected_effect": {
      "treatment": "SomeEffect"
    },
    "formula": null,
    "alpha": 0.05,
    "skip": true,
    "passed": null,
    "result": {
      "status": "skipped",
      "reason": "Test marked as skip:true in the causal test config file.
    }
}  

@f-allian f-allian requested a review from jmafoster1 February 2, 2026 12:40
Copy link
Collaborator

@jmafoster1 jmafoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice solution. I've suggested a couple of potential minor improvements

f-allian and others added 3 commits February 3, 2026 15:47
Co-authored-by: Michael Foster <13611658+jmafoster1@users.noreply.github.com>
Co-authored-by: Michael Foster <13611658+jmafoster1@users.noreply.github.com>
@f-allian f-allian requested a review from jmafoster1 February 4, 2026 15:02
Copy link
Collaborator

@jmafoster1 jmafoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @f-allian. This looks good to me

@f-allian f-allian merged commit 7c10efe into main Feb 5, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants