Skip to content

test: Add RegExp tests for Next.config.js rewrites#9681

Merged
emrysal merged 7 commits intomainfrom
test/next-js-rewrite-regex
Jun 21, 2023
Merged

test: Add RegExp tests for Next.config.js rewrites#9681
emrysal merged 7 commits intomainfrom
test/next-js-rewrite-regex

Conversation

@hariombalhara
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara commented Jun 21, 2023

What does this PR do?

Why these tests

  • Helpful when writing complex Regular Expressions to test out all the cases and properly documenting them.
  • Not all features have their e2e tests and making a change in rewrite sometimes break a feature which doesn't have an e2e. e.g. Workflows got broken due to a RegExp change, because it didn't have any test yet it didn't get caught.

How does it work

  • Works by manually modifying Next.js rewrite RegExps(using simple steps) to make them a valid named capturing group JS RegExp and then writing tests for them

Will we need these tests in future

  • Booker routes are scattered and not a single route and they are not easy to target at the moment if we want to do something with them. So, we might need this in future
  • Org rewrites are coming already and could be helpful for them too cc @roae

What these tests are not

  • These are not a replacement for Checkly tests that should verify on Vercel that a route is working or not. Checkly should cover all routes that are behind rewrites or can be indirectly affected by rewrites.
    cc @ericrommel if you are up for those tests. I am happy writing tests too. I love writing tests

Note: I have intentionally avoiding doing automatic conversion to valid JS RegExp because that itself is complex. Steps to convert manually are straightforward and written in the code.

Type of change

  • Tests

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 21, 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 Jun 21, 2023 0:28am
cal ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 21, 2023 0:28am
web-staging 🛑 Canceled (Inspect) Jun 21, 2023 0:28am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
ui ⬜️ Ignored (Inspect) Visit Preview Jun 21, 2023 0:28am

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 21, 2023

Thank you for following the naming conventions! 🙏

@hariombalhara hariombalhara force-pushed the test/next-js-rewrite-regex branch from c5e491d to 282cb2a Compare June 21, 2023 09:46
@hariombalhara hariombalhara changed the title Add RegExp tests Add RegExp tests for Next.config.js rewrites Jun 21, 2023
@hariombalhara hariombalhara marked this pull request as ready for review June 21, 2023 09:47
@hariombalhara hariombalhara changed the title Add RegExp tests for Next.config.js rewrites test: Add RegExp tests for Next.config.js rewrites Jun 21, 2023
Comment thread apps/web/next.config.js
plugins.push(withAxiom);

/** Needed to rewrite public booking page, gets all static pages but [user] */
const pages = glob
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.

Needed by tests so extracted it out. next.config.js can't export pages directly

Comment thread apps/web/next.config.js Outdated
Comment on lines +90 to +94
const userTypeRouteRegExp = `/:user((?!${pages.join("/|")}$)[^/]*)/:type((?!book)[^/]+)`;
const teamTypeRouteRegExp = "/team/:slug/:type((?!book)[^/]+)";
const privateLinkRouteRegExp = "/d/:link/:slug((?!book)[^/]+)";
const embedUserTypeRouteRegExp = `/:user((?!${pages.join("|")}).*)/:type/embed`;
const embedTeamTypeRouteRegExp = "/team/:slug/:type/embed";
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.

Moved them out in case we need to import these directly and convert to valid JS RegExp automatically.

Comment on lines +11 to +22
// const parts = nextJsRegExp.split(':');

// const validNamedGroupRegExp = parts.map((part, index)=>{
// if (index === 0) {
// return part;
// }
// if (part.match(/^[a-zA-Z0-9]+$/)) {
// return `(?<${part}>[^/]+)`
// }
// part = part.replace(new RegExp('([^(]+)(.*)'), '(?<$1>$2)');
// return part
// }).join('');
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.

Code for automatic conversion. It is tricky in itself. So, no magic in tests for now.

Comment thread apps/web/test/lib/next-config.test.ts Outdated
// - /:user/ -> (?<user>[^/]+)
// - /:user(?!404)[^/]+/ -> (?<user>((?!404)[^/]+))

// userTypeRouteRegExp = `/:user((?!${pages.join("/|")})[^/]*)/:type((?!book)[^/]+)`;
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.

Added the next.js regex here for reference so that they can be easily compared.

Comment on lines +38 to +39
// - /:user/ -> (?<user>[^/]+)
// - /:user(?!404)[^/]+/ -> (?<user>((?!404)[^/]+))
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.

Things to do when doing the conversion

Comment thread apps/web/test/lib/next-config.test.ts Outdated
@hariombalhara hariombalhara requested review from a team, emrysal, keithwillcode and zomars June 21, 2023 09:58
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 21, 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 Jun 21, 2023

Current Playwright Test Results Summary

✅ 114 Passing - ⚠️ 5 Flaky

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

(Last updated on 06/21/2023 12:36:34pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: f23cf33

Started: 06/21/2023 12:33:20pm UTC

⚠️ Flakes

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests should be able to reschedule
Retry 2Retry 1Initial Attempt
17.41% (35) 35 / 201 runs
failed over last 7 days
79.10% (159) 159 / 201 runs
flaked over last 7 days

📄   packages/app-store/routing-forms/playwright/tests/basic.e2e.ts • 3 Flakes

Top 1 Common Error Messages

null

3 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Routing Forms Zero State Routing Forms should be able to edit the form
Retry 1Initial Attempt
6.22% (13) 13 / 209 runs
failed over last 7 days
0.96% (2) 2 / 209 runs
flaked over last 7 days
Routing Forms Seeded Routing Form Routing Link - Reporting and CSV Download
Retry 1Initial Attempt
2.04% (4) 4 / 196 runs
failed over last 7 days
18.37% (36) 36 / 196 runs
flaked over last 7 days
Routing Forms Seeded Routing Form Router URL should work
Retry 1Initial Attempt
0% (0) 0 / 196 runs
failed over last 7 days
2.04% (4) 4 / 196 runs
flaked over last 7 days

📄   apps/web/playwright/booking-pages.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
pro user -- new-booker can reschedule a booking
Retry 1Initial Attempt
0% (0) 0 / 312 runs
failed over last 7 days
0.64% (2) 2 / 312 runs
flaked over last 7 days

View Detailed Build Results


Copy link
Copy Markdown
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

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

💯

@emrysal emrysal merged commit 820b2da into main Jun 21, 2023
@emrysal emrysal deleted the test/next-js-rewrite-regex branch June 21, 2023 15:06
@PeerRich PeerRich added the core area: core, team members only label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants