-
Notifications
You must be signed in to change notification settings - Fork 198
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
Run k6 against local Nuxt on PRs #4924
Conversation
cecca58
to
0bac141
Compare
Latest k6 run output1
Footnotes
|
Okay, so that summary doesn't work, whoops 😅 I'll have to find another way to capture it. |
2f13a67
to
102c8c4
Compare
Okay, the tests are getting throttled when they make API calls. That makes sense. Because these are only local tests meant to test the frontend (and to ensure the tests themselves work), I think it's appropriate to incorporate talkback here. I'll generate new tapes, and have a commit soon to add that. |
c8a0f27
to
c41a5f5
Compare
c41a5f5
to
427a280
Compare
59a408a
to
a480cf9
Compare
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.
I tried running these tests locally, and all of the fa
locale tests have failed because the /fa
url shows the "This content has disappeared" error page. Does it run fine for you?
I remember I added the fa
locale for RTL instead of the ar
locale to these tests, but don't remember the reason. Will have to look it up.
My main blocker to this PR is the number of lines changed. I know those are due to the added tapes, so I opened a PR to only save the first 200 peaks of the audio response.
Yes, but you need to install production translations (which the CI step does using Thanks for the change to limit the peaks, that's perfect, I've merged that into this PR. |
Another method of reducing the tape size, is we could minify parts of them, or perhaps the entire tape. It's nice for them to be inspectable when that's needed, but really we could probably leave them minified by default... It would certainly make the line counts less surprising when adding new tapes! |
Remove reliance on system dictionary which may not be available
54630d6
to
c19150f
Compare
I do use the tape contents sometimes, e.g. when trying to find which id is being opened from the search page - to make sure that the tape exists, or to add exactly that tape.
Oh, I didn't realize it was using the production locales :) |
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.
Awesome, excited to have the tests running soon 🚀
@@ -668,6 +668,86 @@ jobs: | |||
env: | |||
DEPLOYMENT_ENV: production | |||
|
|||
nuxt-load-test: |
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.
Maybe the load testing could be split to a separate action?
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.
Potentially, and it's something I plan to look into when adding the job to run k6 against staging in the next PR after this one. The steps to run k6 on a local environment are so different than running it against a remote environment, I wanted to make sure I didn't abstract too early 🙂
I'll push a change to use We can also opt to make locales configurable if we want to run against more or different locales in live environments. |
Fixes
Part of https://github.com/WordPress/openverse-infrastructure/issues/1031 by @obulat
Depends on and is blocked by #4908.
Description
Run k6 against a locally running Nuxt in PRs. This acts as an integration test of the k6 tests themselves, as well as an informal performance test of the local Nuxt application (albeit, without constraints or anything to normalise the results).
I'm not comfortable yet failing this check if any of the k6 requests fail, because I don't know how reliable it will be to run these tests in the GitHub runner with its specific resources constraints. We can evaluate that in this PR and add it now or later if we like.
Testing Instructions
Check out the new CI step in this PR and see that it makes sense. It should leave a comment with the k6 text summary.
Checklist
Update index.md
).main
) or a parent feature branch.ov just catalog/generate-docs
for catalogPRs) or the media properties generator (
ov just catalog/generate-docs media-props
for the catalog or
ov just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin