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.

Sign up for GitHub

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

chore: enable serverActions in test and e2e studios #6311

Closed
wants to merge 6 commits into from
Closed

chore: enable serverActions in test and e2e studios #6311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7bbb196
chore: update e2e tests to use serverActions
cngonzalez Apr 10, 2024
21bfe63
Merge branch 'next' into sdx-1239
cngonzalez Apr 15, 2024
4c0c617
Merge branch 'next' into sdx-1239
cngonzalez Apr 18, 2024
6982415
chore: enable server actions in test studio and e2e studio
cngonzalez Apr 18, 2024
e0303e6
Merge branch 'next' into sdx-1239
cngonzalez Apr 18, 2024
40a8d45
fix: update mutate check with actions
cngonzalez Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions dev/studio-e2e-testing/sanity.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,7 @@ export default defineConfig({

plugins: [sharedSettings()],
basePath: '/test',
unstable_serverActions: {
enabled: true,
},
})
6 changes: 3 additions & 3 deletions dev/test-studio/sanity.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,9 @@ export default defineConfig([
plugins: [sharedSettings()],
basePath: '/test',
icon: SanityMonogram,
// unstable_serverActions: {
// enabled: true,
// },
unstable_serverActions: {
enabled: true,
},
},
{
name: 'partialIndexing',
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/tests/inputs/text.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ test.describe('inputs: text', () => {
await page.waitForSelector('data-testid=field-simple', {timeout: 30000})
const field = page.getByTestId('field-simple').getByRole('textbox')

// Enter initial text and wait for the mutate call to be sent
Copy link
Contributor

Choose a reason for hiding this comment

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

does this mean we are only testing the actions API and not the mutate API? Shouldn't we test both for a while at least?

Copy link
Member Author

Choose a reason for hiding this comment

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

This particular line seems to be geared just to help this test await the right conditions (rather than testing the endpoint itself).

I agree with your concerns about testing both for a while -- I don't want the team to merge in code that works with server actions but not mutate (which is still the default) and for us to miss an issue.

Maybe what makes the most sense is having our CI stick with the mutate actions, and those of us who are working with server actions can test the e2e tests when we make changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

With that in mind, suggest we close this in favor of #6425 -- I think we've at least proven that e2e tests work as they are with server actions enabled! 🎉

const response = page.waitForResponse(/mutate/)
// Enter initial text and wait for the actions call to be sent
const response = page.waitForResponse(/actions/)
await field.fill(kanji)
await response

Expand Down
Loading