Skip to content

Commit

Permalink
Revert of Revert "test(ssr-tests): print SSR Logs for errors in SSR t…
Browse files Browse the repository at this point in the history
…ests, ALSO when a test times out (#19302) (#19326)

Original PR: #19302
Revert of original PR: #19319 becasue of errors on the `develop`'s pipeline https://github.com/SAP/spartacus/actions/runs/11142085195/job/30964328791

Now bringing back the contents of the original PR, but with ensuring SSR Tests don't encounter the following error again on the pipeline:
```
node:internal/event_target:1099
  process.nextTick(() => { throw err; });
                           ^
Error: Unknown worker message type message
This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
Please open an issue with this stack trace at https://github.com/nodejs/node/issues
    at Function.fail (node:internal/assert:20:9)
    at Worker.[kOnMessage] (node:internal/worker:354:12)
    at MessagePort.<anonymous> (node:internal/worker:232:57)
    at _ZoneDelegate.Object.<anonymous>._ZoneDelegate.invokeTask (/<project>/node_modules/zone.js/bundles/zone.umd.js:445:35)
    (... rest of stacktrace ...)
    at MessagePort.<anonymous> (node:internal/per_context/messageport:23:28) {
  code: 'ERR_INTERNAL_ASSERTION'
```

**How fixed the problem:**
Removed `jest-preset-angular` from the Jest setup of SSR Tests. Used simply `ts-jest` instead.

**Why it helped**
I only deduce that the new custom `testEvironment` introduced in the Original PR that extends from `NodeEnvironment` from `jest-environment-node` **conflicted** with the `jest-preset-angular` which under the hood assumes simulating Browser environment (not Node).

**Why we really don't need or want `jest-preset-angular` in SSR Tests project**
- it enforces `jsdom` testEnvironment (see https://github.com/thymikee/jest-preset-angular/blob/ff0895f4b8dfa561c4f4bc9779016d9d4d7213c7/src/presets/index.ts#L6)
- it enforces importing zone.js (see https://github.com/thymikee/jest-preset-angular/blob/4c2b674634b64eed267e7c0ddb715f52e7839cba/setup-jest.js#L1-L2)
- it enforces initializing Jest test environment with Angular's `BrowserDynamicTestingModule` (see https://github.com/thymikee/jest-preset-angular/blob/4c2b674634b64eed267e7c0ddb715f52e7839cba/setup-jest.js#L18)
- in SSR Tests we want to simply run NodeJS processes, but not simulate Angular components in a browser

Note: Finally I could reproduce it on local too, while using JS `testEnvironment` file instead of TS. Moreover, I could reproduce it only on the first run of tests on local. On the second run, everything would pass. Only when I renamed the file with `testEvironment`, on the first run of tests, the error could be reproduced again.


related to https://jira.tools.sap/browse/CXSPA-8564
  • Loading branch information
Platonn authored Oct 3, 2024
1 parent bc29bb5 commit ce5047c
Show file tree
Hide file tree
Showing 9 changed files with 129 additions and 129 deletions.
2 changes: 2 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@
"jasmine-core": "~5.1.2",
"jasmine-marbles": "^0.9.2",
"jest": "^29.0.0",
"jest-circus": "^29.0.0",
"jest-environment-node": "^29.0.0",
"jest-preset-angular": "13.1.6",
"jsonc-parser": "~3.2.1",
"karma": "~6.4.1",
Expand Down Expand Up @@ -238,4 +240,4 @@
"webpack": "~5.94.0",
"webpack-cli": "^5.0.0"
}
}
}
4 changes: 3 additions & 1 deletion projects/schematics/src/dependencies.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
},
"ssr-tests": {
"@spartacus/core": "2211.29.0-2",
"http-proxy": "^1.18.1"
"http-proxy": "^1.18.1",
"jest-circus": "^29.0.0",
"jest-environment-node": "^29.0.0"
},
"storefrontapp-e2e-cypress": {},
"@spartacus/storefront": {
Expand Down
12 changes: 2 additions & 10 deletions projects/ssr-tests/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,14 @@
const { pathsToModuleNameMapper } = require('ts-jest');
const { compilerOptions } = require('./tsconfig.json');
const { defaultTransformerOptions } = require('jest-preset-angular/presets');

/** @type {import('ts-jest/dist/types').JestConfigWithTsJest} */
module.exports = {
preset: 'jest-preset-angular',
moduleNameMapper: pathsToModuleNameMapper(compilerOptions.paths || {}, {
prefix: '<rootDir>/',
}),
setupFilesAfterEnv: ['<rootDir>/setup-jest.ts'],
testMatch: ['**/+(*.)+(spec).+(ts)'],
transform: {
'^.+\\.(ts|js|mjs|html|svg)$': [
'jest-preset-angular',
{
...defaultTransformerOptions,
tsconfig: '<rootDir>/tsconfig.json',
},
],
'^.+\\.(ts|js|mjs)$': ['ts-jest'],
},

collectCoverage: false,
Expand All @@ -31,4 +22,5 @@ module.exports = {
lines: 90,
},
},
testEnvironment: './src/environments/custom-test-environment.ts',
};
4 changes: 3 additions & 1 deletion projects/ssr-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
},
"peerDependencies": {
"@spartacus/core": "2211.29.0-2",
"http-proxy": "^1.18.1"
"http-proxy": "^1.18.1",
"jest-circus": "^29.0.0",
"jest-environment-node": "^29.0.0"
}
}
9 changes: 0 additions & 9 deletions projects/ssr-tests/setup-jest.ts

This file was deleted.

61 changes: 61 additions & 0 deletions projects/ssr-tests/src/environments/custom-test-environment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* SPDX-FileCopyrightText: 2024 SAP Spartacus team <[email protected]>
*
* SPDX-License-Identifier: Apache-2.0
*/

import { Event, State } from 'jest-circus';
import { TestEnvironment as NodeEnvironment } from 'jest-environment-node';
import { getRawLogsPretty } from '../utils/log.utils';

/**
* This is a custom Jest environment that adds the SSR logs to the test errors.
* The logs are added to the error `cause` property for failed tests,
* so those logs will be printed by Jest along with the test error.
*/
export default class CustomTestEnvironment extends NodeEnvironment {
// For comparison, see the original source code of Jest-Circus:
// https://github.com/jestjs/jest/blob/bd1c6db7c15c23788ca3e09c919138e48dd3b28a/packages/jest-circus/src/formatNodeAssertErrors.ts#L45
async handleTestEvent(event: Event, _state: State) {
if (event.name === 'test_done') {
event.test.errors = event.test.errors.map((errors) => {
if (Array.isArray(errors)) {
const [originalError, asyncError] = errors;

const ssrLogs = getSsrLogs();
// Error's `cause` property is the only property printed by Jest
// besides `message` that we can utilize for attaching logs.
// No other custom properties are printed by Jest.
// See their source code of their function `formatExecError`:
// https://github.com/jestjs/jest/blob/bd1c6db7c15c23788ca3e09c919138e48dd3b28a/packages/jest-message-util/src/index.ts#L436
addCauseToError({
error: originalError, // e.g. error when an expect() fails
cause: ssrLogs,
});
addCauseToError({
error: asyncError, // e.g. error when a test timeout happens
cause: ssrLogs,
});
}
return errors;
});
}
}
}

function addCauseToError({ error, cause }: { error: any; cause: any }) {
// in some cases, the error might be a string, not an object
if (
typeof error === 'object' &&
error !== null &&
'message' in error &&
'stack' in error
) {
error.cause = cause;
}
}

function getSsrLogs() {
const readableLogs = getRawLogsPretty().join('\n');
return `(more context below)\n--- SSR LOGS (with JSONs pretty-printed) ---\n${readableLogs}\n--- SSR LOGS END ---`;
}
122 changes: 55 additions & 67 deletions projects/ssr-tests/src/ssr-testing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,73 +23,61 @@ describe('SSR E2E', () => {
await SsrUtils.startSsrServer();
});

it(
'should receive success response with request',
LogUtils.attachLogsToErrors(async () => {
backendProxy = await ProxyUtils.startBackendProxyServer({
target: BACKEND_BASE_URL,
});
const response: any = await HttpUtils.sendRequestToSsrServer({
path: REQUEST_PATH,
});
expect(response.statusCode).toEqual(200);
it('should receive success response with request', async () => {
backendProxy = await ProxyUtils.startBackendProxyServer({
target: BACKEND_BASE_URL,
});
const response: any = await HttpUtils.sendRequestToSsrServer({
path: REQUEST_PATH,
});
expect(response.statusCode).toEqual(200);

const logsMessages = LogUtils.getLogsMessages();
expect(logsMessages).toContain(`Rendering started (${REQUEST_PATH})`);
expect(logsMessages).toContain(
`Request is waiting for the SSR rendering to complete (${REQUEST_PATH})`
);
})
);
const logsMessages = LogUtils.getLogsMessages();
expect(logsMessages).toContain(`Rendering started (${REQUEST_PATH})`);
expect(logsMessages).toContain(
`Request is waiting for the SSR rendering to complete (${REQUEST_PATH})`
);
});

it(
'should receive response with 404 when page does not exist',
LogUtils.attachLogsToErrors(async () => {
backendProxy = await ProxyUtils.startBackendProxyServer({
target: BACKEND_BASE_URL,
});
const response = await HttpUtils.sendRequestToSsrServer({
path: REQUEST_PATH + 'not-existing-page',
});
expect(response.statusCode).toEqual(404);
})
);
it('should receive response with 404 when page does not exist', async () => {
backendProxy = await ProxyUtils.startBackendProxyServer({
target: BACKEND_BASE_URL,
});
const response = await HttpUtils.sendRequestToSsrServer({
path: REQUEST_PATH + 'not-existing-page',
});
expect(response.statusCode).toEqual(404);
});

it(
'should receive response with status 404 if HTTP error occurred when calling cms/pages API URL',
LogUtils.attachLogsToErrors(async () => {
backendProxy = await ProxyUtils.startBackendProxyServer({
target: BACKEND_BASE_URL,
callback: (proxyRes, req) => {
if (req.url?.includes('cms/pages')) {
proxyRes.statusCode = 404;
}
},
});
const response = await HttpUtils.sendRequestToSsrServer({
path: REQUEST_PATH,
});
expect(response.statusCode).toEqual(404);
})
);
it('should receive response with status 404 if HTTP error occurred when calling cms/pages API URL', async () => {
backendProxy = await ProxyUtils.startBackendProxyServer({
target: BACKEND_BASE_URL,
callback: (proxyRes, req) => {
if (req.url?.includes('cms/pages')) {
proxyRes.statusCode = 404;
}
},
});
const response = await HttpUtils.sendRequestToSsrServer({
path: REQUEST_PATH,
});
expect(response.statusCode).toEqual(404);
});

it.skip(
'should receive response with status 500 if HTTP error occurred when calling other than cms/pages API URL',
LogUtils.attachLogsToErrors(async () => {
backendProxy = await ProxyUtils.startBackendProxyServer({
target: BACKEND_BASE_URL,
callback: (proxyRes, req) => {
if (req.url?.includes('cms/components')) {
proxyRes.statusCode = 404;
}
},
});
const response = await HttpUtils.sendRequestToSsrServer({
path: REQUEST_PATH,
});
expect(response.statusCode).toEqual(500);
})
);
it.skip('should receive response with status 500 if HTTP error occurred when calling other than cms/pages API URL', async () => {
backendProxy = await ProxyUtils.startBackendProxyServer({
target: BACKEND_BASE_URL,
callback: (proxyRes, req) => {
if (req.url?.includes('cms/components')) {
proxyRes.statusCode = 404;
}
},
});
const response = await HttpUtils.sendRequestToSsrServer({
path: REQUEST_PATH,
});
expect(response.statusCode).toEqual(500);
});
});

describe('With caching enabled', () => {
Expand All @@ -99,7 +87,7 @@ describe('SSR E2E', () => {

it(
'should take the response from cache for the next request if previous render succeeded',
LogUtils.attachLogsToErrors(async () => {
async () => {
backendProxy = await ProxyUtils.startBackendProxyServer({
target: BACKEND_BASE_URL,
});
Expand All @@ -123,13 +111,13 @@ describe('SSR E2E', () => {
expect(logsMessages2).toContain(
`Render from cache (${REQUEST_PATH})`
);
}),
},
2 * SsrUtils.DEFAULT_SSR_TIMEOUT // increase timeout for this test as it calls the SSR server twice
);

it(
'should render for the next request if previous render failed',
LogUtils.attachLogsToErrors(async () => {
async () => {
backendProxy = await ProxyUtils.startBackendProxyServer({
target: BACKEND_BASE_URL,
callback: (proxyRes, req) => {
Expand All @@ -152,7 +140,7 @@ describe('SSR E2E', () => {
expect(logsMessages).not.toContain(
`Render from cache (${REQUEST_PATH})`
);
}),
},
2 * SsrUtils.DEFAULT_SSR_TIMEOUT // increase timeout for this test as it calls the SSR server twice
);
});
Expand Down
40 changes: 0 additions & 40 deletions projects/ssr-tests/src/utils/log.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,43 +126,3 @@ export async function waitUntilLogContainsText(
);
});
}

/**
* A higher-order function that wraps a test callback and includes SSR logs
* in any error thrown during the test execution. The logs are put into the `cause`
* property of the Error.
*
* @param testFn - The original test function to be wrapped.
* @returns A new function that can be passed to Jest's `it()` or `test()`.
*
* @example
* it('should perform SSR correctly', attachLogsToErrors(async () => {
* // Your test code here
* }));
*/
export function attachLogsToErrors(
testFn: () => Promise<void> | void
): () => Promise<void> {
return async () => {
try {
await testFn();
} catch (error: unknown) {
const readableLogs = getRawLogsPretty().join('\n');
const ssrLogs = `(more context below)\n--- SSR LOGS (with JSONs pretty-printed) ---\n${readableLogs}\n--- SSR LOGS END ---`;

if (error instanceof Error) {
// Error's `cause` property is the only property printed by Jest
// besides `message` that we can utilize for attaching logs.
// No other custom properties are printed by Jest.
// See their source code of their function `formatExecError`:
// https://github.com/jestjs/jest/blob/bd1c6db7c15c23788ca3e09c919138e48dd3b28a/packages/jest-message-util/src/index.ts#L436

error.cause = ssrLogs;
} else {
throw new Error(error as string, { cause: ssrLogs });
}

throw error;
}
};
}

0 comments on commit ce5047c

Please sign in to comment.