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

Update tox to version 4 #1592

Merged
merged 3 commits into from
Mar 12, 2023
Merged

Conversation

greghope667
Copy link
Contributor

@greghope667 greghope667 commented Feb 26, 2023

Summary of changes

Updates tox.ini configuration to support building with newer tox 4.
This is not backwards compatible with tox version 3.

Pull Request Checklist

  • Changes have tests
  • News fragment added in news.d. See documentation for details

I've successfully build & run tests on Linux, this will need testing on Windows and Mac

@Robberduckzilla
Copy link
Contributor

Looks great! Thank you for the PR, G!

@Robberduckzilla
Copy link
Contributor

@greghope667 would you mind also adding a news fragment? it's needed to merge a PR, apparently.
in news.d/bugfix/, if you add a file called 1592.core.md and in that, put a 1 sentence description of the changes
e.g. updated config from supporting tox3 to tox4

I'll test the changes on windows & mac just now.
Thank you once again for the awesome work!

@greghope667
Copy link
Contributor Author

I've added a news fragment now, thanks.

Tests pass on Linux. Windows has some failing tests for me, but they also fail for me on the master branch so I don't think this change is the culprit.

@greghope667 greghope667 marked this pull request as ready for review February 27, 2023 23:01
@mkrnr
Copy link
Contributor

mkrnr commented Mar 10, 2023

Updating tox will remove one obstacle when setting up the dev environment, very nice!
@greghope667 looks like you cancelled the Github actions run? I did a quick test (#1596), the actions should finish successfully. Could you trigger the actions again?

Tomorrow I'll test your changes on macOS, Windows ARM, and Linux ARM. Let's get this PR merged! 💪

Edit: Looks like it's not possible to trigger a re-run here? In the worst case you can do a force-push to trigger another run.

@mkrnr
Copy link
Contributor

mkrnr commented Mar 11, 2023

Tested successfully:

  • Windows 11 ARM
    • tox r -e launch
  • Mac OS (ARM but with x86_64 compatibility mode)
    • arch -x86_64 tox r -e test
    • arch -x86_64 tox r -e py3,py39
    • arch -x86_64 tox r -e setup -- bdist_app
    • arch -x86_64 tox r -e plugins_install -- plover-mod-z
    • arch -x86_64 tox r -e launch

Tested partially successful:

  • Windows 11 ARM
    • tox r -e test: Failed with the same tests as the master (as already observed by Greg):
      • test\gui_qt\test_dictionaries_widget.py ..................................FFF

Tested unsuccessfully:

  • Windows 11 ARM
    • tox r -e setup -- bdist_win
      • Can't get that to work at all but same issue on the master. Local issue...
  • Mac OS (ARM but with x86_64 compatibility mode)
    • arch -x86_64 tox r -e packaging_checks
      • packaging_checks: commands[0]> '{[helpers]functions.sh}' -- packaging_checks packaging_checks: exit 2 (0.01 seconds) /Users/martin/git/plover-greghope667> '{[helpers]functions.sh}' -- packaging_checks .pkg: _exit> python /Users/martin/git/plover/venv/lib/python3.9/site-packages/pyproject_api/_backend.py True setuptools.build_meta packaging_checks: FAIL code 2 (15.98=setup[15.98]+cmd[0.01] seconds) evaluation failed :( (16.12 seconds)
      • This can be fixed by replacing functions.sh = in tox.ini line 6 with functions = as well as all usages of this variable (line 82 and 93). This also fixes the release_prepare command
  • Linux ARM
    • Not able to test anything because of PyQt5 :( ...should be fine since you tested Linux yourself. Will buy a computer with Intel chip to install Linux on next days...

If you apply the fix for functions.sh = in tox.ini (see above) and all Github Actions are successful, this PR is good to go from my side.

@greghope667
Copy link
Contributor Author

Hi @mkrnr , thanks for looking at the PR and running tests. I've just added the functions.sh fix.

The CI job failed (but showed as cancelled) due to deprecation by GitHub - fixed by #1597

@sammdot
Copy link
Member

sammdot commented Mar 12, 2023

Confirmed that packaging check now passes after the latest change.

I tried to run on Mac with native ARM as well and it seemed to not work at all, which if I understand correctly may have to do with PyQt5, as on Linux. We're possibly doing a migration to PyQt6 soon anyway, so for now this will do.

@sammdot
Copy link
Member

sammdot commented Mar 12, 2023

Looks good. Thank you all!

@sammdot sammdot merged commit 9057504 into openstenoproject:master Mar 12, 2023
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.

4 participants