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 workflow to run e2e tests from lxd-ui #14035

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

edlerd
Copy link
Contributor

@edlerd edlerd commented Sep 3, 2024

Added a step to the GitHub test workflow, executing the e2e test suite from lxd-ui with a lxd backend built from the current branch from a PR or main branch.

We might extend this on the stable-5.21 and stable-5.0 branches, as we have dedicated test suites in lxd-ui for those. For older versions, we should continue skipping the e2e tests. I'd keep this as a second step in a followup PR.

This was successful on a fork, i.e. see this run.

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Hi @edlerd please can you work with @simondeziel to update this to use the pre-built LXD binaries we now have (that are compiled during the Code Tests phase) and to use the re-usable workflows for performance tuning and dependency installation.

Also i'd like to avoid building the docs twice, but they are built into the tarball i believe during the Code Tests phase so may be available already now.

@edlerd edlerd force-pushed the add-e2e-tests branch 17 times, most recently from 6f77ba3 to df58339 Compare November 13, 2024 19:07
@edlerd edlerd requested a review from tomponline November 13, 2024 21:22
@edlerd
Copy link
Contributor Author

edlerd commented Nov 13, 2024

@simondeziel @tomponline Updated the workflow to consume the build artefacts from previous steps. Please give it another review.

@edlerd
Copy link
Contributor Author

edlerd commented Nov 13, 2024

Also using the docs artefact by depending and downloading on that build step.

@edlerd edlerd force-pushed the add-e2e-tests branch 2 times, most recently from 57f706e to b56aaf2 Compare November 25, 2024 16:57
@edlerd edlerd requested a review from tomponline November 26, 2024 12:55
@edlerd
Copy link
Contributor Author

edlerd commented Nov 26, 2024

I think all issues and concerns raised above have been addressed. Please give this another go @tomponline and @simondeziel

@edlerd
Copy link
Contributor Author

edlerd commented Nov 26, 2024

This run on my fork was passing and producing coverage data for the LXD-UI tests.

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

I think we're pretty close with this one now.

@tomponline
Copy link
Member

@simondeziel happy with this one?

@edlerd edlerd force-pushed the add-e2e-tests branch 2 times, most recently from 8673c99 to e3fc5da Compare November 26, 2024 14:30
- name: Install LXD-UI dependencies
run: |
set -eux
sudo chmod 0777 ./lxd-ui
Copy link
Member

Choose a reason for hiding this comment

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

out of interest how come this is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, this is pretty old code and I don't remember the exact details. I assume dotrun is creating files and folders under a different user, and we need to give it permission to do so.

@edlerd edlerd requested a review from tomponline November 28, 2024 10:22
@tomponline tomponline merged commit 7dc90e0 into canonical:main Nov 28, 2024
25 checks passed
@tomponline
Copy link
Member

We might extend this on the stable-5.21 and stable-5.0 branches, as we have dedicated test suites in lxd-ui for those. For older versions, we should continue skipping the e2e tests. I'd keep this as a second step in a followup PR.

My understanding is that the LXD-UI from main should work on stable-5.21 and stable-5.0 branches, so can I just backport this test as part of our normal backport process?

@tomponline
Copy link
Member

@edlerd looks like we missed blocking the edge builds on the ui tests passing

https://github.com/canonical/lxd/pull/14035/files#diff-1db27d93186e46d3b441ece35801b244db8ee144ff1405ca27a163bfe878957fL507

@edlerd
Copy link
Contributor Author

edlerd commented Nov 28, 2024

We might extend this on the stable-5.21 and stable-5.0 branches, as we have dedicated test suites in lxd-ui for those. For older versions, we should continue skipping the e2e tests. I'd keep this as a second step in a followup PR.

My understanding is that the LXD-UI from main should work on stable-5.21 and stable-5.0 branches, so can I just backport this test as part of our normal backport process?

Basically yes.

The problem is the test suite needs to be adjusted for older versions. You can backport it, but need to adjust the command

npx playwright test --project chromium:lxd-latest-edge

to

npx playwright test --project chromium:lxd-5.21-edge

or

npx playwright test --project chromium:lxd-5.0-edge

to run the right tests for the version.

@tomponline
Copy link
Member

The problem is the test suite needs to be adjusted for older versions. You can backport it, but need to adjust the command npx playwright test --project chromium:lxd-latest-edge to npx playwright test --project chromium:lxd-5.21-edge or npx playwright test --project chromium:lxd-5.0-edge to run the right tests for the version.

Please can you update so this is automatic using logic like this:

https://github.com/canonical/lxd/blob/main/.github/workflows/tests.yml#L683-L687

@edlerd
Copy link
Contributor Author

edlerd commented Nov 28, 2024

Please can you update so this is automatic using logic like this:

https://github.com/canonical/lxd/blob/main/.github/workflows/tests.yml#L683-L687

Addressing this and other comments from above in the follow up PR #14553

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.

3 participants