-
Notifications
You must be signed in to change notification settings - Fork 352
chore(catalyst): CATALYST-1297 Run E2E tests on PRs #2822
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
Conversation
🦋 Changeset detectedLatest commit: 885bddb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
c8b3c91 to
1734c76
Compare
dd40301 to
16d78ef
Compare
16d78ef to
b96c61a
Compare
b96c61a to
83ba600
Compare
83ba600 to
abd05fc
Compare
abd05fc to
08cb116
Compare
08cb116 to
bbe4811
Compare
bbe4811 to
31fbf0c
Compare
31fbf0c to
3127077
Compare
3127077 to
2e09c1b
Compare
2e09c1b to
5efeb89
Compare
5efeb89 to
b3efeac
Compare
b3efeac to
8d31c4d
Compare
954ae17 to
f332816
Compare
f332816 to
848ef51
Compare
848ef51 to
c77b89f
Compare
c77b89f to
c694fd6
Compare
chanceaclark
left a comment
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.
Two 🍹's but looks good on my end.
.github/workflows/basic.yml
Outdated
| - name: Build catalyst-client | ||
| run: pnpm --filter @bigcommerce/catalyst-client build | ||
|
|
||
| - name: Build catalyst-core | ||
| run: pnpm --filter @bigcommerce/catalyst-core build |
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.
🍹 Could we just run pnpm build and let turbo cache the other projects? We'll need to provide the turbo env var here I think to enable the build cache, but allows us to not worry about each thing that needs to the built.
| - name: Run Tests | ||
| run: pnpm run test | ||
|
|
||
| e2e-tests: |
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.
🍹 What about instead of defining each job manually, we use a matrix to run the tests. I threw AI to showcase what I mean on this branch (not sure it actually works though): CATALYST-1297...refactor/e2e-tests
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 promising! I'll try it.
jorgemoya
left a comment
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.
What an :mvp:!
de89294 to
83ebaba
Compare
83ebaba to
b2a9873
Compare
b2a9873 to
2fbef6d
Compare
2fbef6d to
885bddb
Compare
What/Why?
Ensure Catalyst E2E tests are executed on pull requests. This will help prevent further breaking changes from contributions. There are 3 separate jobs:
E2E Functional Tests- Executes all e2e testsE2E Functional Tests (alternate locale)- Executes all tests that require an alternate locale to be usedE2E Functional Tests (TRAILING_SLASH=false)- Executes the functional tests that require theTRAILING_SLASHenv var to be disabled. This will prevent 301 redirect loop regressionsTesting
Test suites executed on this PR and they are passing.
Migration
Add
requiredprop to the Country selector and Country input fields incore/vibes/soul/sections/cart/shipping-form/index.tsxon lines 280 and 28For all other changes, use this PR as a reference.