Skip to content
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): Validate http port 4000 is not used by other process, before running tests #19337

Merged
merged 15 commits into from
Oct 4, 2024

Conversation

Platonn
Copy link
Contributor

@Platonn Platonn commented Oct 4, 2024

Problem:
Each SSR test starts and kills an SSR process in the background, listening on port 4000.
If you interrupt the tests that you run on local (e.g. by pressing Cmd+C), then the Jest tests process is stopped but the SSR process might be still dangling in the background.
So the next time you try to re-run SSR Tests, they'll fail with because the new SSR process cannot be started on the already occupied port 4000.

Previosuly, the we had to wait for all tests fail (30secons), until we were presented with an vague error message about port being in use. Moreover it didn't get any hint how to solve the problem.

Now, the problem is detected before running any test. The error message is descriptive and provides a hint how to kill a process that is listening on the conflicting port.

Note: Instead of detecting the problem and showing an useful message, my first strategy was to avoid the problem in general. I mean that whenever the parent Jest process is interupted, I wanted the child SSR process to be killed as well. I've tried hooks like process.on('SIGTERM') etc. , I've tried tampering the config of spawning the child process (remove detach: true, add killSignal) or calling child.unref(), but I had various problems - the child process was still not killed when parent was interupted or the child process couldn't be killed with process.kill() in the tests body

QA steps

  1. Build SSR npm build:libs && npm build && npm build:local-http-backend

  2. In one terminal run npm run serve:ssr

  3. In second terminal run npm run test:ssr

  4. Verify the following error message is presented in the second terminal:

    BEFORE:

    image

    AFTER:

    image

  5. Kill the SSR process in the first terminal (e.g. using the commands from the advice from the console)

  6. Re-run ssr tests in the second terminal and verity they pass

Fixes https://jira.tools.sap/browse/CXSPA-8587

@Platonn Platonn changed the base branch from develop to feature/CXSPA-8586 October 4, 2024 11:35
@Platonn Platonn changed the title test(SSR e2e) Validate http port 4000 is not used by other process, before running tests test(SSR e2e): Validate http port 4000 is not used by other process, before running tests Oct 4, 2024
@pawelfras pawelfras marked this pull request as ready for review October 4, 2024 14:44
@pawelfras pawelfras requested a review from a team as a code owner October 4, 2024 14:44
@pawelfras
Copy link
Contributor

Full pipeline run with SSR tests: https://github.com/SAP/spartacus/actions/runs/11182025201

pawelfras
pawelfras previously approved these changes Oct 4, 2024
Copy link
Contributor

@pawelfras pawelfras left a comment

Choose a reason for hiding this comment

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

LGTM. QA done

Base automatically changed from feature/CXSPA-8586 to develop October 4, 2024 15:36
@Platonn Platonn dismissed pawelfras’s stale review October 4, 2024 15:36

The base branch was changed.

@github-actions github-actions bot marked this pull request as draft October 4, 2024 15:38
@Platonn Platonn marked this pull request as ready for review October 4, 2024 15:40
Copy link

cypress bot commented Oct 4, 2024

spartacus    Run #45149

Run Properties:  status check passed Passed #45149  •  git commit a865c49d8c: test(SSR e2e): before running tests, check if some process listens on the port 4...
Project spartacus
Branch Review feature/CXSPA-8587
Run status status check passed Passed #45149
Run duration 00m 45s
Commit git commit a865c49d8c: test(SSR e2e): before running tests, check if some process listens on the port 4...
Committer Krzysztof Platis
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 22
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 805
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

Copy link

cypress bot commented Oct 4, 2024

spartacus    Run #45153

Run Properties:  status check passed Passed #45153  •  git commit 3a78c81992 ℹ️: Merge c5a41e3d2c2d30f843665436f0c3b7cae341c05c into 81c7302ba0da80a2e5feb3423281...
Project spartacus
Branch Review feature/CXSPA-8587
Run status status check passed Passed #45153
Run duration 13m 22s
Commit git commit 3a78c81992 ℹ️: Merge c5a41e3d2c2d30f843665436f0c3b7cae341c05c into 81c7302ba0da80a2e5feb3423281...
Committer Krzysztof Platis
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 4
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 125
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎

@Platonn Platonn merged commit 7d650b3 into develop Oct 4, 2024
28 checks passed
@Platonn Platonn deleted the feature/CXSPA-8587 branch October 4, 2024 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants