Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ORCA-253] Return Failure Reasons to suites.json and Failed Tests to output.csv #56

Merged
merged 31 commits into from
May 10, 2024

Conversation

BWMac
Copy link
Contributor

@BWMac BWMac commented May 3, 2024

Problem:

Currently, the results provided by DCQC to users in the output file are limited to the dcqc_status for each file in the original manifest. Additionally, there is no status indicating that a test was unable to be performed on a file due to an error encountered during the execution of an external test.

Scope:

This PR adds functionality to capture failure reasons for both internal and external tests, but errors and error reasons for external tests only. I also encountered a complication with external tests where many of them will return the same exit_code whether they have failed or errored out. This will be addressed in future work by adding some more complex/specific logic to assess a "true" failure vs. an error. For now, these errors show up as failures, and the error_reason is captured instead as failure_reason.

Solution:

To surface more information about DCQC runs to users, the following changes were made:

  • Implemented logic for capturing failure reasons for both external and internal tests.
    • For external tests, this involved updates to the ExternalTestMixin and each individual external test class to capture information about how each test should be interpreted.
    • For internal tests, this involved adding failure reason strings at each place in the test logic where a test failure would be determined.
    • As mentioned above, errors and error reasons were also captured for external tests only.
    • Implemented logic to surface which tests were required, skipped, failed, and errored for each file in the final DCQC output file.

Testing:

  • All necessary tests and test files were updated in the repository to accommodate these changes.
  • New testing conditions were added for capturing the internal test failure reasons where needed.
  • The changes above were tested on Nextflow Tower.
    • This run was used to test a set of files to determine if the overall logic was working.
    • This run was used to test the suite error by intentionally providing a bad file path to the TiffTag306DateTimeTest
    • Both test runs used a new test_full configuration that I created to test all file types that we support. The files used are housed here on Synapse.

Notes:

  • Bumped fs-synapse to 2.0 (synapseclient version 4.1.1).
  • Updated CONTRIBUTING.md.
  • Added DPE as code owners.

Copy link

swarmia bot commented May 3, 2024

@BWMac BWMac changed the title [ORCA-253] Return Failure Reasons and Failed Tests to output.csv [ORCA-253] Return Failure Reasons to suites.json and Failed Tests to output.csv May 9, 2024
@BWMac BWMac marked this pull request as ready for review May 9, 2024 17:21
@BWMac BWMac requested review from thomasyu888 and BryanFauble May 9, 2024 17:21
CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Contributor

@BryanFauble BryanFauble left a comment

Choose a reason for hiding this comment

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

Logic looks great to me, just a few comments.

@BWMac
Copy link
Contributor Author

BWMac commented May 9, 2024

Thanks for the suggestions @BryanFauble! I made those changes in ba4d83f and ab39fcf.

Copy link
Contributor

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

🔥 LGTM! Excellent work and review!

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
120 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@BWMac BWMac merged commit 1febf2e into main May 10, 2024
11 checks passed
@BWMac BWMac deleted the bwmac/orca-256/failure_prototyping branch May 10, 2024 17:31
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.

3 participants