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

Add image cache testing for Mac and Windows #2716

Merged
merged 4 commits into from
Jun 1, 2022
Merged

Conversation

akaszynski
Copy link
Member

@akaszynski akaszynski commented May 30, 2022

Add in image cache testing for MacOS and Windows based on the comment in #2441.

Changes:

  • Removed cache for test_plot as we're using full_screen=True here and this does not work with unit testing within MacOS. The full_screen parameter actually changes the window since, while on Linux it does not.
  • Several tests fail on Windows when using (and not using) OpenGL compiled with OSMesa. Rather than having a separate test suite with these images, this PR just skips them.

@github-actions github-actions bot added the maintenance Low-impact maintenance activity label May 30, 2022
@codecov
Copy link

codecov bot commented May 30, 2022

Codecov Report

Merging #2716 (b0a6a86) into main (975645d) will decrease coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2716      +/-   ##
==========================================
- Coverage   93.82%   93.76%   -0.07%     
==========================================
  Files          75       75              
  Lines       16148    16148              
==========================================
- Hits        15151    15141      -10     
- Misses        997     1007      +10     

@akaszynski akaszynski changed the title add in mac image cache testing add in image cache testing for Mac and Windows May 30, 2022
@akaszynski akaszynski changed the title add in image cache testing for Mac and Windows Add image cache testing for Mac and Windows May 30, 2022
@adeak
Copy link
Member

adeak commented May 30, 2022

Just so I understand correctly, this means that we don't need a separate image cache for each OS after all, at the cost of skipping this small handful of flaky tests?

@akaszynski
Copy link
Member Author

Just so I understand correctly, this means that we don't need a separate image cache for each OS after all, at the cost of skipping this small handful of flaky tests?

Exactly. MacOS only needed one change, Windows would have just needed two skips but for OSMesa. 90% testing is better than no testing.

Copy link
Member

@adeak adeak left a comment

Choose a reason for hiding this comment

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

Looks good to me as-is, thanks. One very small suggestion that's not critical at all.

tests/plotting/test_plotting.py Outdated Show resolved Hide resolved
Copy link
Member

@adeak adeak left a comment

Choose a reason for hiding this comment

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

Thanks!

@akaszynski akaszynski merged commit 6eb0b23 into main Jun 1, 2022
@akaszynski akaszynski deleted the ci/mac_os_image_cache branch June 1, 2022 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Low-impact maintenance activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants