Update single-binary.yml end2end_execute #6061
Conversation
Signed-off-by: taieeuu <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6061 +/- ##
==========================================
- Coverage 37.10% 37.09% -0.01%
==========================================
Files 1318 1318
Lines 132331 132403 +72
==========================================
+ Hits 49100 49119 +19
- Misses 78959 79011 +52
- Partials 4272 4273 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: taieeuu <[email protected]>
Signed-off-by: taieeuu <[email protected]>
taieeuu
changed the title
| # assert flytesnacks_release_tag == "v1.0.0" | ||
| # assert config_file_path == "/path/to/config.yaml" |
|
|
||
| if __name__ == "__main__": | ||
| cli() | ||
| return results |
There was a problem hiding this comment.
Tests shouldn't really return anything. Can you instead assert on results?
There was a problem hiding this comment.
Okay ! Thank you for your review ~
| @@ -152,7 +148,6 @@ def run( | |||
| ) | |||
| r = requests.get(manifest_url) | |||
| parsed_manifest = r.json() | |||
| workflow_groups = [] | |||
There was a problem hiding this comment.
why are we removing this?
There was a problem hiding this comment.
I noticed that the workflow_groups parameter is immediately reassigned in the next line, so I just think it might not need to be repeated.
| "test_project_name": pytestconfig.getoption("--test_project_name"), | ||
| "test_project_domain": pytestconfig.getoption("--test_project_domain"), | ||
| "cluster_pool_name": pytestconfig.getoption("--cluster_pool_name"), | ||
| } |
There was a problem hiding this comment.
nit: can you add a newline?
There was a problem hiding this comment.
okay ! Thank you for your review. ~
| if return_non_zero_on_failure: | ||
| for result in results: | ||
| if result["status"] not in ("passing", "coming soon"): | ||
| sys.exit(1) | ||
|
|
||
| pytest.fail("Workflow execution failed") |
There was a problem hiding this comment.
what happens if multiple workflows fail?
There was a problem hiding this comment.
I changed it to run all the tests and then print out any error messages.
Signed-off-by: taieeuu <[email protected]>
Issue
issue_6075
Why are the changes needed?
It currently calls a Makefile and then runs a script to execute Python. Ideally, we should replace make end2end_execute with a direct pytest command.
What changes were proposed in this pull request?
How was this patch tested?
github action passed
Check all the applicable boxes