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

[QA] VizroAI UI tests #882

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

[QA] VizroAI UI tests #882

wants to merge 40 commits into from

Conversation

l0uden
Copy link
Contributor

@l0uden l0uden commented Nov 18, 2024

Description

Moved VizroAI UI tests from vizro-qa

Notice

  • I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

    • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
    • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
    • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
    • I have not referenced individuals, products or companies in any commits, directly or indirectly.
    • I have not added data or restricted code in any commits, directly or indirectly.

@github-actions github-actions bot added the Vizro-AI 🤖 Issue/PR that addresses Vizro-AI package label Nov 18, 2024
@l0uden l0uden changed the base branch from main to qa/component_library_tests November 18, 2024 12:13
Copy link
Contributor

github-actions bot commented Nov 18, 2024

View the example dashboards of the current commit live on PyCafe ☕ 🚀

Updated on: 2024-12-12 12:42:53 UTC
Commit: bb0b680

Link: vizro-core/examples/dev/

Link: vizro-core/examples/scratch_dev

Link: vizro-core/examples/visual-vocabulary/

Link: vizro-ai/examples/dashboard_ui/

@l0uden l0uden changed the base branch from qa/component_library_tests to main November 18, 2024 12:14
@l0uden l0uden changed the base branch from main to qa/component_library_tests November 18, 2024 12:20
Base automatically changed from qa/component_library_tests to main November 26, 2024 11:11
l0uden and others added 5 commits December 12, 2024 13:41
@l0uden l0uden marked this pull request as ready for review December 16, 2024 11:01
Copy link
Contributor

@maxschulz-COL maxschulz-COL left a comment

Choose a reason for hiding this comment

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

Thanks @l0uden , added a few comments

@@ -48,6 +48,7 @@ prep-release = [
]
pypath = "hatch run python -c 'import sys; print(sys.executable)'"
test = "pytest tests {args}"
test-e2e-vizro-ai-ui = "pytest -vs --reruns 1 tests/e2e/test_vizro_ai_ui.py --headless {args}"
Copy link
Contributor

Choose a reason for hiding this comment

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

If we add this as a hatch script, does it run locally if I was to use that command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, anyway to use it you need to run the VIzroAI UI first

@@ -56,6 +57,7 @@ test-unit-coverage = [
"- coverage combine",
"coverage report"
]
vizro-ai-ui = "python examples/dashboard_ui/app.py &"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this hatch command really necessary? Could this not work like the other examples in vizro-core where the user can specify the example by providing args

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Couldn't do it. Let's take a look a this way together

]
norecursedirs = ["tests/tests_utils"]
Copy link
Contributor

Choose a reason for hiding this comment

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

What does that do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wanted to do it as in vizro-core repo, but looks like here it doesn't have any sense. Thanks. Deleted

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be better to have a test_utils also per folder, like here for e2e? Then you could save yourself also all the e2e file prefixes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My point was to make same folder structure as in vizro-core for the tests.

@@ -0,0 +1,204 @@
#!/usr/bin/env bash

#The MIT License (MIT)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have a license here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took this file from the repo which has MIT license, in this case we should specify it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Vizro-AI 🤖 Issue/PR that addresses Vizro-AI package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants