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

Address ion-test-driver failure by upgrading pip with setup-python action. #875

Merged
merged 3 commits into from
Jun 12, 2024

Conversation

nirosys
Copy link
Contributor

@nirosys nirosys commented Jun 6, 2024

Issue #, if available: #795

Description of changes:
It appears that the issue causing the ion-test-driver failure is related to an issue found in pip 22.0. This PR adds the setup-python action to force a pip upgrade (and ensure python version).

A run of the workflow can be seen here.

In addition to the setup-python change, I also had to roll-back the revision of ion-test-driver that was being used to before the 1.0 and 1.1 split was made. The workflow was pulling master, which has been using the directory split for a while, but it appears the iontests used by this repo is from before versioned directories were added.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

codecov bot commented Jun 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.67%. Comparing base (3c1b6b1) to head (ed96200).
Report is 54 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #875      +/-   ##
============================================
+ Coverage     67.23%   67.67%   +0.43%     
- Complexity     5484     5522      +38     
============================================
  Files           159      159              
  Lines         23025    22982      -43     
  Branches       4126     4108      -18     
============================================
+ Hits          15481    15553      +72     
+ Misses         6262     6155     -107     
+ Partials       1282     1274       -8     

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

@nirosys nirosys force-pushed the rgiliam/workflow_python_setup branch from 3cf9041 to c8e1567 Compare June 6, 2024 22:13
@nirosys nirosys marked this pull request as ready for review June 6, 2024 22:42
@nirosys nirosys requested a review from tgregg June 6, 2024 22:42
Copy link
Contributor

@tgregg tgregg left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for fixing this.

Note that we recently changed ion-tests to restore the old iontestdata directory: amazon-ion/ion-tests#93

Maybe we can just make a quick update to ion-test-driver to point the 1.0 branch at iontestdata so we can keep pointing at master here? (see amazon-ion/ion-test-driver@1815f9c)

@nirosys
Copy link
Contributor Author

nirosys commented Jun 6, 2024

Yea, I'll put a PR together to update ion-test-driver to match the ion-tests change. I'll try to get that together before EoD, then I can set this PR back to use master.

@nirosys nirosys merged commit f350aa3 into amazon-ion:master Jun 12, 2024
10 checks passed
@tgregg tgregg mentioned this pull request Jun 12, 2024
linlin-s pushed a commit that referenced this pull request Jul 3, 2024
…tion. (#875)

* Use setup-python to force pip upgrade; fixes error with pip 22.0

* Roll ion-test-driver back to pre-1.1/1.0 split

* Revert ion-test-driver reference back to master

Co-authored-by: Tyler Gregg <[email protected]>

---------

Co-authored-by: Tyler Gregg <[email protected]>
linlin-s pushed a commit that referenced this pull request Jul 3, 2024
…tion. (#875)

* Use setup-python to force pip upgrade; fixes error with pip 22.0

* Roll ion-test-driver back to pre-1.1/1.0 split

* Revert ion-test-driver reference back to master

Co-authored-by: Tyler Gregg <[email protected]>

---------

Co-authored-by: Tyler Gregg <[email protected]>
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