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

Fix tests #302

Merged
merged 2 commits into from
Jan 5, 2023
Merged

Fix tests #302

merged 2 commits into from
Jan 5, 2023

Conversation

talonchandler
Copy link
Collaborator

@talonchandler talonchandler commented Jan 5, 2023

The tests on #297 are failing, and the issue seems to be related to DISPLAY XAUTHORITY, which is a tox setting I picked up from the napari plugin cookiecutter.

This PR removes this line and the tests seem to pass (with this change alone, and in the #297 branch).

These docs make me think this setting is for testing GUI elements, which we currently aren't doing in GH actions.

@ziw-liu, do you think this is safe to merge?

Run GabrielBB/xvfb-action@v[1](https://github.com/mehta-lab/recOrder/pull/297/checks#step:7:1)
  with:
    run: python -m tox
  env:
    pythonLocation: /Users/runner/hostedtoolcache/Python/[3](https://github.com/mehta-lab/recOrder/pull/297/checks#step:7:3).8.15/x6[4](https://github.com/mehta-lab/recOrder/pull/297/checks#step:7:4)
    PLATFORM: macos-latest
/Users/runner/hostedtoolcache/Python/3.8.1[5](https://github.com/mehta-lab/recOrder/pull/297/checks#step:7:5)/x[6](https://github.com/mehta-lab/recOrder/pull/297/checks#step:7:6)4/bin/python -m tox
py3[8](https://github.com/mehta-lab/recOrder/pull/297/checks#step:7:9)-macos: failed with pass_env values cannot contain whitespace, use comma to have multiple values in a single line, invalid values found 'DISPLAY XAUTHORITY'
  py38-macos: FAIL code 1 (0.01 seconds)
  evaluation failed :( (0.43 seconds)
Error: The process '/Users/runner/hostedtoolcache/Python/3.8.15/x64/bin/python' failed with exit code 1

@deprecated-napari-hub-preview-bot
Copy link

deprecated-napari-hub-preview-bot bot commented Jan 5, 2023

Preview page for your plugin is ready here:
https://preview.napari-hub.org/mehta-lab/recOrder/302
Updated: 2023-01-05T04:37:46.993127

@talonchandler talonchandler changed the title Fix tox tests? Fix tests Jan 5, 2023
@talonchandler talonchandler marked this pull request as ready for review January 5, 2023 02:40
@ziw-liu
Copy link
Contributor

ziw-liu commented Jan 5, 2023

These docs make me think this setting is for testing GUI elements, which we currently aren't doing in GH actions.

You are right: the environment variables DISPLAY and XAUTHORITY are related to having a X-session running on the Linux workers.

Actually napari had the same issue recently: napari/napari#5441

@ziw-liu ziw-liu added bug Something isn't working testing Testing the code maintenance labels Jan 5, 2023
@ziw-liu
Copy link
Contributor

ziw-liu commented Jan 5, 2023

Seems like copying the fix in napari/napari@7fee6f4 works. Functionally the same since we're not using the feature yet.

@talonchandler
Copy link
Collaborator Author

Great find @ziw-liu!

Ready to merge from my perspective.

@codecov-commenter
Copy link

Codecov Report

Merging #302 (37cadc7) into main (655a764) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##            main    #302   +/-   ##
=====================================
  Coverage   4.27%   4.27%           
=====================================
  Files         23      23           
  Lines       5102    5102           
=====================================
  Hits         218     218           
  Misses      4884    4884           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working maintenance testing Testing the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants