-
Notifications
You must be signed in to change notification settings - Fork 74
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-294 - Fix Safe-Tests(ctl-not-external) on CI is timing out #5568
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
fides Run #11460
Run Properties:
|
Project |
fides
|
Branch Review |
refs/pull/5568/merge
|
Run status |
Passed #11460
|
Run duration | 00m 59s |
Commit |
e1281c3dda ℹ️: Merge 6cae869d1f3f47557f9922b8ef9765a2b61d9c76 into 9bbab10f1de89f652ac95e41e754...
|
Committer | Andres Torres |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5568 +/- ##
==========================================
+ Coverage 79.11% 87.11% +8.00%
==========================================
Files 408 388 -20
Lines 24870 23904 -966
Branches 2704 2585 -119
==========================================
+ Hits 19675 20825 +1150
+ Misses 4676 2522 -2154
- Partials 519 557 +38 ☔ View full report in Codecov by Sentry. |
@@ -145,13 +145,16 @@ def test_parse(test_config_path: str, test_cli_runner: CliRunner) -> None: | |||
|
|||
|
|||
class TestDB: | |||
@pytest.mark.skip( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eastandwestwind here I added the skip, now the CI is green.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow up ticket https://ethyca.atlassian.net/browse/HJ-317
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing blocking, minor question above. Thanks!
@pytest.mark.integration | ||
def test_reset_db(self, test_config_path: str, test_cli_runner: CliRunner) -> None: | ||
result = test_cli_runner.invoke( | ||
cli, ["-f", test_config_path, "db", "reset", "-y"] | ||
) | ||
print(result.output) | ||
assert result.exit_code == 0 | ||
assert result.exit_code == 0, result.output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this asserting exactly? Can we make this a separate assert
line for clarity?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is checking for the result.exit_code
to be success (0), and if it fails, it prints the result.output
. Am I answering the right question?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, thanks for clarifying!
fides Run #11462
Run Properties:
|
Project |
fides
|
Branch Review |
main
|
Run status |
Passed #11462
|
Run duration | 00m 54s |
Commit |
ad6a9c31af: HJ-294 - Fix Safe-Tests(ctl-not-external) on CI is timing out (#5568)
|
Committer | Andres Torres |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
Closes #HJ-294
Description Of Changes
Fix Safe-Tests(ctl-not-external) on CI is timing out
Code Changes
configure_db
to re-raise errors (they were being obscured before), and now they are being handled properly by/admin/db/{action}
endpoint.Steps to Confirm
Pre-Merge Checklist
CHANGELOG.md
updatedmain
downgrade()
migration is correct and works