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

HJ-300 - Timed out tests should fail on CI #5571

Merged
merged 4 commits into from
Dec 9, 2024

Conversation

andres-torres-marroquin
Copy link
Contributor

Closes #HJ-300

Description Of Changes

Timed out tests should fail on CI, they are now actually failing on timeout.

Code Changes

  • Removed continue-on-error for Safe-Tests

Steps to Confirm

  1. Removed continue-on-error for Safe-Tests

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
  • Followup issues:
    • Followup issues created (include link)
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

Copy link

vercel bot commented Dec 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Dec 6, 2024 4:24pm

@andres-torres-marroquin
Copy link
Contributor Author

@adamsachs mentioned on #5563

this looks good, and may be a fine starting point - but probably worth considering removing this flag from other jobs in the workflow?

I think we we can keep some of the other ones in place, we currently have:

  • Static-Checks
  • Performance-Checks
  • Check-Container-Startup

Copy link

cypress bot commented Dec 6, 2024

fides    Run #11326

Run Properties:  status check passed Passed #11326  •  git commit f57b79b5f3 ℹ️: Merge 83faf44feec124b99d810936df1e6ff21f6e9db2 into bd7bd4bfcc2f4a4a3bf72514c636...
Project fides
Branch Review refs/pull/5571/merge
Run status status check passed Passed #11326
Run duration 01m 06s
Commit git commit f57b79b5f3 ℹ️: Merge 83faf44feec124b99d810936df1e6ff21f6e9db2 into bd7bd4bfcc2f4a4a3bf72514c636...
Committer Andres Torres
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
View all changes introduced in this branch ↗︎

Copy link

codecov bot commented Dec 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.02%. Comparing base (bd7bd4b) to head (83faf44).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5571   +/-   ##
=======================================
  Coverage   79.02%   79.02%           
=======================================
  Files         389      389           
  Lines       24722    24722           
  Branches     2702     2702           
=======================================
+ Hits        19536    19537    +1     
  Misses       4665     4665           
+ Partials      521      520    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

looks good, let's just keep our eyes out for two things related to this, moving forward:

  1. whether there are any unintended side effects of switching this off, i.e. maybe it was in place originally for a legitimate reason that we haven't considered. i think it should be fine, but just wanna state this so we can both keep our eyes out!
  2. whether it may make sense to switch this off for any of the other CI jobs it's currently enabled for. maybe it's fine to keep, per your comment - but let's just keep that possibility in the back of our minds!

@andres-torres-marroquin andres-torres-marroquin merged commit d848f4b into main Dec 9, 2024
36 of 39 checks passed
@andres-torres-marroquin andres-torres-marroquin deleted the andres/HJ-300 branch December 9, 2024 16:29
Copy link

cypress bot commented Dec 9, 2024

fides    Run #11356

Run Properties:  status check passed Passed #11356  •  git commit d848f4b63f: HJ-300 - Timed out tests should fail on CI (#5571)
Project fides
Branch Review main
Run status status check passed Passed #11356
Run duration 00m 38s
Commit git commit d848f4b63f: HJ-300 - Timed out tests should fail on CI (#5571)
Committer Andres Torres
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 4
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

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