-
Notifications
You must be signed in to change notification settings - Fork 153
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
[QA] Move integration tests to vizro #950
Conversation
…_integration_tests_to_vizro
for more information, see https://pre-commit.ci
View the example dashboards of the current commit live on PyCafe ☕ 🚀Updated on: 2025-02-04 14:28:51 UTC Link: vizro-core/examples/dev/ Link: vizro-core/examples/scratch_dev |
…om/mckinsey/vizro into qa/move_integration_tests_to_vizro
for more information, see https://pre-commit.ci
…om/mckinsey/vizro into qa/move_integration_tests_to_vizro
for more information, see https://pre-commit.ci
…om/mckinsey/vizro into qa/move_integration_tests_to_vizro
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great to me so far! 🚀 Amazing work, @l0uden! 🥇
That said, I think we should discuss a few key topics on a call rather than communicating everything through PR comments. Moving all the tests from the vizro-qa
repo to the vizro
repo is a significant change and potentially introduces some risks (like unoptimised folder structure...). It’s also a great opportunity to optimise while changes are still manageable and cheap because when the full migration is complete, making similar updates could become much more complex.
What do you think about this @antonymilne @Joseph-Perkins?
To reduce risks and streamline the process, I propose setting up the call about the Vizro tests architecture. Addressing these points now will save time and effort later, and it will help us to merge this PR faster (than communicating these things over the PR comments), allowing @l0uden to proceed with the test migration smoothly.
Some topics/questions I'd like to discuss:
- Should we add a dedicated
test
hatch environment? - Can we improve the folder structure for better maintainability?
- Should we combine
vizro-core/tests/integrations
withvizro-core/tests/e2e
(and apply the same approach tovizro-ai
)?
Thanks @petar-qb ! Completely agree with you about the call about this, let's discuss time on Monday, when @antonymilne will be available. And also it would be better if he could have time to take a look on what's done. |
…_integration_tests_to_vizro
There was a problem hiding this 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 🚀
I have some suggestions, please only implement them if they seem useful, and they make sense!
vizro-core/tests/e2e/vizro/dashboards/default/dashboard_pages.py
Outdated
Show resolved
Hide resolved
vizro-core/tests/tests_utils/e2e/custom_feature_helpers/custom_actions/custom_actions.py
Outdated
Show resolved
Hide resolved
…_integration_tests_to_vizro
Considering all that @maxschulz-COL suggested and everything we aligned on during last week’s call, I’ve prepared the following folder structure proposal. This structure closely resembles what we currently have but introduces the following changes:
@maxschulz-COL @l0uden @antonymilne what do you think about the seven changes I suggested? |
That sounds good to me! Thanks @petar-qb! All perfect except maybe |
…_integration_tests_to_vizro
@maxschulz-COL and @petar-qb, thanks for the amazing comments! I'll try to summarise here the major changes I made.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good now. 🚀
…_integration_tests_to_vizro
…_integration_tests_to_vizro
…_integration_tests_to_vizro
…_integration_tests_to_vizro
Description
Here's the first iteration of tests moving from vizro-qa to vizro repo.
Here I moved (and refactored) part of the default dashboard, test helpers and tests for filters, pages and themes. Also created CI for running the tests
The aim of this PR is to find the way of organising this tests in the proper way, so I can move the rest of them after in the same manner
Notice
I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":