fix: set python container version explicitly #776
Conversation
| @@ -200,7 +201,7 @@ def test_completed_quantum_job(aws_session, capsys): | |||
|
|
|||
|
|
|||
| @pytest.mark.xfail( | |||
| (sys.version_info.major, sys.version_info.minor) != decorator_python_version, | |||
| (sys.version_info.major, sys.version_info.minor) != (3, 10), | |||
There was a problem hiding this comment.
I'm not sure I understand why the previous setup doesn't work. Could you please add more details on what is causing that to fail?
There was a problem hiding this comment.
Yes, the fixture doesn't get evaluated inside the decorator, so this comparison is something like (3, 10) != <pytest.fixture-decorator_python_version>
Not sure how this didn't get caught in its pr, but there was a lot of back and forth with the method for skipping and looks like the final version missed the mark
There was a problem hiding this comment.
Does it necessarily need to be a fixture? Can it be instead set to a regular variable?
| # image_uri = retrieve_image(Framework.BASE, aws_session.region) | ||
| # tag = aws_session.get_full_image_tag(image_uri) | ||
| # major_version, minor_version = re.search(r"-py(\d)(\d+)-", tag).groups() | ||
| # return major_version, minor_version |
There was a problem hiding this comment.
I'll capture a follow-up to go back to getting the python version from the container in the code, but setting manually to 3.10 to unblock
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #776 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 129 129
Lines 8360 8360
Branches 1862 1862
=========================================
Hits 8360 8360 ☔ View full report in Codecov by Sentry. |
|
Error is fixed with latest code change, it's still the input links bug |
|
I have confirmed that integ tests PASS with 3.10 on the latest commit and have verified skipping with 3.9 since the last change to the xfail decorators |
|
Reverified passing tests since last commit |
| @@ -244,6 +244,8 @@ def decorator_job(a, b: int, c=0, d: float = 1.0, **extras): | |||
| with open("test/output_file.txt", "w") as f: | |||
| f.write("hello") | |||
|
|
|||
| return job_test_script.job_helper() | |||
There was a problem hiding this comment.
switched this test to use the return method for more variety
| @@ -304,6 +305,7 @@ def decorator_job_submodule(): | |||
| ) as f: | |||
| assert f.readlines() == ["pytest\n"] | |||
| assert dir(pytest) | |||
| save_job_result(submodule_helper()) | |||
There was a problem hiding this comment.
moved this line lower so if something above fails the results assert will fail
Good call, I'll capture a task to add this |
Issue #, if available:
Description of changes:
Previous set up was skipping even when version matched
Testing done:
integ test passing with correct python version and skipping with incorrect
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.