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

e2e tests: use webbrowser to check for urls in viewer #5818

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

isabelizimm
Copy link
Contributor

@isabelizimm isabelizimm commented Dec 18, 2024

addresses #5569

@:viewer

I don't think that the idea of printing the url after the API is started is super feasible on the vetiver side (see rstudio/vetiver-python#219). I have an alternative test instead where we open up an arbitrary blank webpage. AFAICT the main purpose of this test is to make sure the viewer is intercepting webbrowser calls and opening them in the Viewer pane, so this will still check for all those tasks. LMK if this seems like a reasonable option!

Can also probably pull vetiver as a dependency, as webbrowser is a builtin Python module (which is a second bonus, since vetiver has a few large dependencies 😆)

QA Notes

should pass CI

@isabelizimm isabelizimm requested a review from midleman December 18, 2024 22:07
Copy link

github-actions bot commented Dec 18, 2024

E2E Tests 🚀  ?
This PR will run tests tagged with: @critical @viewer

@isabelizimm
Copy link
Contributor Author

@:viewer

@midleman
Copy link
Contributor

@:viewer

just add this anywhere in your PR description. backticks are not required...

@isabelizimm
Copy link
Contributor Author

isabelizimm commented Dec 19, 2024

Updated, looks like they're running now(?). That is so slick!

@midleman
Copy link
Contributor

Updated, looks like they're running now(?). That is so slick!

Almost. If you added the tag after your test job kicked off, it wouldn't have picked them up. But if you pushed up any new changes or re-run them since the addition, the tests tagged with @viewer will be included.

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