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

Simple GUI test for CI #223

Conversation

GenevieveBuckley
Copy link
Collaborator

@GenevieveBuckley GenevieveBuckley commented Oct 4, 2023

Closes #205

This PR:

  1. Updates the github test.yaml workflow file to enable GUI testing on CI (as described here), and
  2. Adds a very simple GUI test which opens the 2D annotator dock widgets in napari. We don't interact with it or do any actual testing of the annotation functions. This GUI test just creates the image embedding and then opens the dock widgets in the napari viewer, as if it was ready to start.

That brings our code coverage up to 23%. I did do some extra work (not in this branch) to see if I could mimic user interaction of annotation to this test... but that only increased the test coverage by about 2%. My guess is that these functions might already be covered by unit tests, so it's not helpful to make a giant integration test anyway.

Careful: this PR is just added straight on top of the many, many git commits involved in branch #214. We should do, uh, something with git to fix any problems from that. (Worst case scenario, I can close and and reopen a new PR for this after #214 is merged.)

Code coverage report is automatically uploaded to codecov.io
https://about.codecov.io/
@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

❗ No coverage uploaded for pull request base (dev@2f58d48). Click here to learn what that means.
The diff coverage is n/a.

@@          Coverage Diff           @@
##             dev     #223   +/-   ##
======================================
  Coverage       ?   23.12%           
======================================
  Files          ?       30           
  Lines          ?     3965           
  Branches       ?        0           
======================================
  Hits           ?      917           
  Misses         ?     3048           
  Partials       ?        0           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@GenevieveBuckley
Copy link
Collaborator Author

I really do not like how the CI job takes <5 minutes on the codecov-report PR branch, but on this branch it is taking over half an hour.
Trying to figure out why it's so slow:

  • I don't think it's installing pyqt and napari, all the steps before we run pytest finish in a reasonable amount of time
  • The pytest step is very, very slow!
    • It's not because my new GUI test is slow - I commented that out in one of the commits, but still found the CI job took an unreasonably long time to run
    • ...I think it might be the use of aganders3/headless-gui? We could test this theory by replacing pytest with echo hello and then look at how long each CI step took to run. I'm not sure what to do if this is the reason the CI is slow, I think we need it for GUI tests.

@GenevieveBuckley GenevieveBuckley changed the base branch from master to dev October 16, 2023 07:13
@GenevieveBuckley
Copy link
Collaborator Author

Update: I think it's the test/test_training.py file that was making the CI so slow. I accidentally opened this PR against the master branch, not dev (not that it helps much, because everything needs to work in master eventually)

A possible workaround would be to mark the super slow tests as pytest.mark.slow and skip them in the CI jobs.

@constantinpape
Copy link
Contributor

Update: I think it's the test/test_training.py file that was making the CI so slow.

Yes, unfortunately test_training.py is quite slow.

A possible workaround would be to mark the super slow tests as pytest.mark.slow and skip them in the CI jobs.

Yes, that's one possibility. Before that I want to check if I can improve this test a bit to speed it up on CI and so that we can just run it with the normal tests.
(I developed the test on my notebook, and it was fast enough, but I guess the CPU in the CI is weaker, so that it takes quite long.)

For now: since you now have the PR on dev it should not be a problem here. Can you go ahead and fix the merge conflicts (which I think are because I squashed and merged #214) and, if you have any more changes you want to make, add them too?

I will then:

  • squash and merge this PR
  • merge dev into master and also update dev, so that we have all the tests up-to-date and on the same branches
  • look into speeding up test_training.py (or mark it as pytest.mark.slow if I can't speed it up without making the test worse)
  • look into adding more GUI tests

@GenevieveBuckley
Copy link
Collaborator Author

I've fixed the merge conflicts, and opened a separate issue for discussion about test_training.py.

I'm very happy that this CI job runs in about 6 minutes - the gui test is not slow at all!

@constantinpape constantinpape merged commit a0a47b3 into computational-cell-analytics:dev Oct 17, 2023
3 checks passed
anwai98 pushed a commit that referenced this pull request Oct 25, 2023
Enable GUI tests in CLI and add a simple GUI test.
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.

Enable GUI tests in CI
2 participants