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

LA-118 - Timed out tests should fail on CI #5563

Closed
wants to merge 3 commits into from

Conversation

andres-torres-marroquin
Copy link
Contributor

@andres-torres-marroquin andres-torres-marroquin commented Dec 5, 2024

Closes #

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 5, 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) Visit Preview Dec 6, 2024 3:45pm

Copy link

cypress bot commented Dec 5, 2024

fides    Run #11318

Run Properties:  status check passed Passed #11318  •  git commit bc2f074e02 ℹ️: Merge 39f975f38e78fe13b1a562269da3082b44c1e98d into ca8e8470f38218fd95d448e1cf64...
Project fides
Branch Review refs/pull/5563/merge
Run status status check passed Passed #11318
Run duration 00m 49s
Commit git commit bc2f074e02 ℹ️: Merge 39f975f38e78fe13b1a562269da3082b44c1e98d into ca8e8470f38218fd95d448e1cf64...
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 ↗︎

@@ -92,7 +92,6 @@ jobs:
'"pytest(nox)"',
]
runs-on: ubuntu-latest
continue-on-error: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may want to keep this one, so it doesn't stop if Static-Checks fail.

@@ -118,7 +117,6 @@ jobs:
Performance-Checks:
needs: Check-Container-Startup
runs-on: ubuntu-latest
continue-on-error: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may want to keep this one, so it doesn't stop if Performance-Checks fail.

@@ -206,7 +204,6 @@ jobs:

runs-on: ubuntu-latest
timeout-minutes: 15
continue-on-error: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want it to continue to check even if Check-Container-Startup failed?

@andres-torres-marroquin andres-torres-marroquin marked this pull request as ready for review December 6, 2024 15:45
@andres-torres-marroquin andres-torres-marroquin requested review from erosselli and adamsachs and removed request for erosselli December 6, 2024 15:54
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.

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

@andres-torres-marroquin andres-torres-marroquin deleted the andres/LA-118 branch December 6, 2024 16:20
@andres-torres-marroquin andres-torres-marroquin restored the andres/LA-118 branch December 6, 2024 16:21
@andres-torres-marroquin andres-torres-marroquin deleted the andres/LA-118 branch December 6, 2024 16:26
@andres-torres-marroquin
Copy link
Contributor Author

Superceded by #5571

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