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

MWPW-160756: Setup Nala tests for studio #116

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

MWPW-160756: Setup Nala tests for studio #116

wants to merge 12 commits into from

Conversation

afmicka
Copy link
Collaborator

@afmicka afmicka commented Dec 13, 2024

Adding setup for Nala tests, github workflow to run on PRs and the very first test

Resolve https://jira.corp.adobe.com/browse/MWPW-160756

Test URLs:

Before: https://main--mas--adobecom.aem.live/studio.html
After: https://MWPW-160756--mas--adobecom.aem.live/studio.html

Copy link

aem-code-sync bot commented Dec 13, 2024

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

Copy link

aem-code-sync bot commented Dec 13, 2024

Page Scores Audits Google
📱 /studio.html PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /studio.html PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync aem-code-sync bot temporarily deployed to MWPW-160756 December 13, 2024 13:57 Inactive
@afmicka afmicka added the do not merge PR should not be merged yet label Dec 13, 2024
@afmicka
Copy link
Collaborator Author

afmicka commented Dec 13, 2024

@3ch023 @npeltier @yesil
The setup here will trigger the tests on every PR. At the moment, logging-in is not happening on github actions (unknown reasons) and the tests would be failing if this PR gets merged.

If MWPW-164257 gets implemented in the meantime, you can apply changes from the branch on my fork: afmicka:MWPW-160756-token that is ready for that implementation (no changes needed assuming the implementation is the same as for the OST in milo).

Otherwise, you can disable the run on PRs (remove workflow file from this PR), merge this as is and be able to run tests locally (that works) and keep adding new ones.

},

{
name: 'mas-live-firefox',
Copy link
Collaborator

Choose a reason for hiding this comment

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

if we don't plan to run studio tests on FF and webkit in nearest future (which i assume we shouldn't) lets just remove them - no need to keep extra code if its unused

# Run Playwright tests on the specific projects using root-level playwright.config.js
# This will be changed later
echo "*** Running tests on specific projects ***"
npx playwright test --config=./playwright.config.js ${TAGS} ${EXCLUDE_TAGS} --project=mas-live-chromium --project=mas-live-firefox --project=mas-live-webkit ${REPORTER} || EXIT_STATUS=$?
Copy link
Collaborator

Choose a reason for hiding this comment

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

i would remove --project=mas-live-firefox --project=mas-live-webkit since we skip them later in test anyway.
nala installs infra each time for them, it makes tests longer without any gain


test.beforeEach(async ({ page, browserName, baseURL }) => {
test.slow();
test.skip(
Copy link
Collaborator

Choose a reason for hiding this comment

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

this can be removed if we adjust npx command to only use chromium project

Copy link
Collaborator

@3ch023 3ch023 left a comment

Choose a reason for hiding this comment

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

  • minor comments/suggestion
  • test account needs to be verified

@@ -0,0 +1,206 @@
#!/usr/bin/env node
Copy link
Collaborator

Choose a reason for hiding this comment

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

we don't have to change it in this PR but i find having both pr.run.sh and nala.run.js doing similar thing confusing, are we sure we need them both? or could be 1 script?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge PR should not be merged yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants