-
Notifications
You must be signed in to change notification settings - Fork 34
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
test(atomic): POC - cache API requests #4468
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -279,6 +279,7 @@ | |
"unexcluded", | ||
"univer", | ||
"Unregisters", | ||
"unroute", | ||
"unsub", | ||
"Unsubscriber", | ||
"uparrow", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import {APIResponse, Request, Route} from '@playwright/test'; | ||
import {createHash} from 'crypto'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I remember you talked about browser compat for crypto. |
||
import {existsSync, mkdirSync, readFileSync, writeFileSync} from 'fs'; | ||
import {join} from 'path'; | ||
|
||
export const cacheFilePath = './playwright/.cache/'; | ||
|
||
export const hashRequest = (url: string, body: string | null) => { | ||
const str = url + body; | ||
return createHash('sha512').update(str).digest('hex'); | ||
}; | ||
|
||
export const cache = async (key: string, response: APIResponse) => { | ||
mkdirSync(cacheFilePath, {recursive: true}); | ||
const body = await response.text(); | ||
// TODO: check if the file already exists and throw | ||
writeFileSync(join(cacheFilePath, key), body); | ||
return body; | ||
}; | ||
|
||
export const cacheHandler = async (route: Route, request: Request) => { | ||
console.log('intercepted ---', request.url()); | ||
const {clientId, ...rest} = request.postDataJSON(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we want to handle search API requests too, I think we could do: const {clientId, analytics, ...rest} = request.postDataJSON(); although this might be too broad... another option would be to filter out non-static params case-by-case, based on the request.url |
||
const key = hashRequest(request.url(), JSON.stringify(rest)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Are you using the same authentication for each requests? Two exact request with the same payload may have different response depending on the authentication used. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point! IIRC it's the same authentication for all test suites. I don't foresee the need of changing the authentication for specific tests since we target a public endpoint. |
||
const cachedFile = join(cacheFilePath, key); | ||
|
||
if (existsSync(cachedFile)) { | ||
console.log('---reading from cache'); | ||
await route.fulfill({ | ||
body: readFileSync(cachedFile, 'utf8'), | ||
}); | ||
} else { | ||
console.log('---fetching from network'); | ||
const response = await route.fetch(); | ||
await route.fulfill({ | ||
response, | ||
body: await cache(key, response), | ||
}); | ||
} | ||
}; |
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.
We also currently have playwright tests that call non-commerce API endpoints, so we'd have to modify this accordingly.
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.
Yes. We also need to handle request to the listing endpoint.