Skip to content

chore: make Embed and AppStore Tests required#10358

Merged
keithwillcode merged 2 commits into
mainfrom
chore/make-embed-app-store-tests-required
Jul 25, 2023
Merged

chore: make Embed and AppStore Tests required#10358
keithwillcode merged 2 commits into
mainfrom
chore/make-embed-app-store-tests-required

Conversation

@hariombalhara
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara commented Jul 25, 2023

What does this PR do?

Make all tests required now.
A recent regression that could have been avoided if embed tests were required #10357

Side Note: Marked it High Priority as it's quick to merge and super important to have these tests block PR merge considering recent regression as well.

@hariombalhara hariombalhara requested a review from zomars July 25, 2023 08:41
@vercel
Copy link
Copy Markdown

vercel Bot commented Jul 25, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2023 9:57am
cal-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2023 9:57am
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 25, 2023 9:57am
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Jul 25, 2023 9:57am
qa ⬜️ Ignored (Inspect) Jul 25, 2023 9:57am
ui ⬜️ Ignored (Inspect) Visit Preview Jul 25, 2023 9:57am

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 25, 2023

Thank you for following the naming conventions! 🙏

@zomars zomars added the core area: core, team members only label Jul 25, 2023
@hariombalhara hariombalhara added High priority Created by Linear-GitHub Sync and removed core area: core, team members only labels Jul 25, 2023
Comment thread .github/workflows/pr.yml

e2e-embed-react:
name: E2E React embeds tests
if: ${{ needs.changes.outputs.embed-react == 'true' }}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Now there is no condition to run these checks.

Comment thread .github/workflows/pr.yml

required:
needs: [lint, type-check, test, build, e2e]
needs: [lint, type-check, test, build, e2e, e2e-embed, e2e-embed-react, e2e-app-store]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As there is no condition on these checks, we can require them here.

Copy link
Copy Markdown
Contributor

@keithwillcode keithwillcode left a comment

Choose a reason for hiding this comment

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

@hariombalhara to avoid adding more minutes of wait time back to builds, let’s implement the matrix strategy like we have on the main e2e test suite.

@keithwillcode keithwillcode added automated-tests area: unit tests, e2e tests, playwright core area: core, team members only labels Jul 25, 2023
@keithwillcode keithwillcode added this to the v3.2 milestone Jul 25, 2023
@keithwillcode keithwillcode requested a review from a team July 25, 2023 08:47
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 25, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Jul 25, 2023

Current Playwright Test Results Summary

✅ 74 Passing - ⚠️ 1 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 07/25/2023 10:03:28am UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 2e951b2

Started: 07/25/2023 10:01:54am UTC

⚠️ Flakes

📄   apps/web/playwright/webhook.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
FORM_SUBMITTED can submit a form and get a submission event
Retry 1Initial Attempt
0.70% (2) 2 / 284 runs
failed over last 7 days
4.58% (13) 13 / 284 runs
flaked over last 7 days

View Detailed Build Results


@alwaysmeticulous
Copy link
Copy Markdown

alwaysmeticulous Bot commented Jul 25, 2023

🤖 Meticulous spotted visual differences in 34 of 231 screens tested: view and approve differences detected.

Last updated for commit 2e951b2. This comment will update as new commits are pushed.

@hariombalhara
Copy link
Copy Markdown
Member Author

@keithwillcode I have pushed the changes for matrix support. Yet to test them. But I noticed that the embed and AppStore tests don't take more time than core e2e tests.
AFAIK, All these tests run in parallel, so even without sharding they would compete much earlier than core tests.

If I am right in that analysis, we don't need sharding.

image

@keithwillcode
Copy link
Copy Markdown
Contributor

IMG_7832

@hariombalhara 13 minutes is what they used to take before I put the matrix in place. Now they only take 2-3 minutes.

@keithwillcode keithwillcode merged commit 3a80c82 into main Jul 25, 2023
@keithwillcode keithwillcode deleted the chore/make-embed-app-store-tests-required branch July 25, 2023 11:37
joeauyeung pushed a commit that referenced this pull request Jul 26, 2023
* Now, all tests are required

* Add matrix support
sean-brydon pushed a commit that referenced this pull request Jul 31, 2023
* Now, all tests are required

* Add matrix support
sean-brydon pushed a commit that referenced this pull request Aug 1, 2023
* Now, all tests are required

* Add matrix support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated-tests area: unit tests, e2e tests, playwright core area: core, team members only High priority Created by Linear-GitHub Sync

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants