-
Notifications
You must be signed in to change notification settings - Fork 385
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(SSR e2e): re-enable test "should receive response with status 500 if HTTP error occurred when calling other than cms/pages API URL" #19331
Conversation
…0 if HTTP error occurred when calling other than cms/pages API URL"
spartacus Run #45135
Run Properties:
|
Project |
spartacus
|
Branch Review |
feature/CXSPA-8570
|
Run status |
Passed #45135
|
Run duration | 57m 22s |
Commit |
5d61fb06c4: refactor: reuse `surroundingLinesRadius` variable
|
Committer | Krzysztof Platis |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
3
|
Pending |
22
|
Skipped |
0
|
Passing |
805
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
…in logs. And provide more context about the problem with parsing
spartacus Run #45132
Run Properties:
|
Project |
spartacus
|
Branch Review |
feature/CXSPA-8570
|
Run status |
Passed #45132
|
Run duration | 12m 05s |
Commit |
dcaae57d40 ℹ️: Merge 5d61fb06c496aefaa77074fdeb29a35e8eb89d8e into 23b72bb0749577b194d3a53c18fe...
|
Committer | Krzysztof Platis |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
3
|
Pending |
2
|
Skipped |
0
|
Passing |
125
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
function validateJsonsInLogs(logs: string[]): void { | ||
logs.forEach((text) => { | ||
if (text.charAt(0) === '{') { | ||
function validateJsonsInLogs(rawLogs: string[]): void { |
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.
medium:
Can you provide steps to verify the warning locally? Btw. the SSR testing framework is getting more complex and maybe we should consider covering by testing at least the most crucial parts of utils
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.
Good question.
After rethinking, I believe we shouldn't validate logs in runtime of tests to detect if the app is built in dev mode. But instead we should check if the app is built in prod mode, before running the tests.
So I reverted changes to logs validation made in this PR.
And in this PR I'm leaving only the un-skip of the test.
Created a separate PR for checking if app is built in prod mode: #19334
… proxy backend before running tests. Previously we were checking the app's behavior in the runtime of tests, checking its symptoms of running in dev mode.
Full pipeline run, with SSR Tests: https://github.com/SAP/spartacus/actions/runs/11181040768 |
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.
QA done.
Note: the change of the intercepted API URL
cms/pages
tocms/components
is not the fix of the flakiness.It's simply reverting an accidental change that was made in the very recent PR - see https://github.com/SAP/spartacus/pull/19307/files?w=1#diff-799b5da094cfd6cc4192ea13d66afac3f264e0502fe21893c3c4e04f024fae45R73 (change from
cms/components
tocms/pages
). This accidental change wasn't spotted before, because this test used to be skipped.Now we're un-skipping it.
The flakiness could have been fix by the PR #19307, which introduced a new way of intercepting requests -
responseInterceptor
(instead of the oldcallback
).I've re-run this test 20+ times on local and it passes.
closes https://jira.tools.sap/browse/CXSPA-8570