Skip to content

Use dynamic port assignment for manual test CI.#20030

Merged
przemyslaw-zan merged 8 commits intomasterfrom
ci/4352
Apr 2, 2026
Merged

Use dynamic port assignment for manual test CI.#20030
przemyslaw-zan merged 8 commits intomasterfrom
ci/4352

Conversation

@przemyslaw-zan
Copy link
Copy Markdown
Member

@przemyslaw-zan przemyslaw-zan commented Mar 31, 2026

🚀 Summary

check-manual-tests.sh now discovers a free port via a Node.js one-liner (binding to port 0), passes it to the server with --port, and forwards it to the web crawler through pnpm run manual:verify --port. The web crawler resolves the port from the --port CLI argument or falls back to 8125. This prevents CI failures when port 8125 is already in use.


📌 Related issues


🧾 Checklists

Use the following checklists to ensure important areas were not overlooked.
This does not apply to feature-branch merges.
If an item is not relevant to this type of change, simply leave it unchecked.

Author checklist

  • Is the changelog entry intentionally omitted?
  • Is the change backward-compatible?
  • Have you considered the impact on different editor setups and core interactions? (e.g., classic/inline/multi-root/many editors, typing, selection, paste, tables, lists, images, collaboration, pagination)
  • Has the change been manually verified in the relevant setups?
  • Does this change affect any of the above?
  • Is performance impacted?
  • Is accessibility affected?
  • Have tests been added that fail without this change (against regression)?
  • Have the API documentation, guides, feature digest, and related feature sections been updated where needed?
  • Have metadata files (ckeditor5-metadata.json) been updated if needed?
  • Are there any changes the team should be informed about (e.g. architectural, difficult to revert in future versions or having impact on other features)?
  • Were these changes documented (in Logbook)?

Reviewer checklist

  • PR description explains the changes and the chosen approach (especially when performance, API, or UX is affected).
  • The changelog entry is clear, user‑ or integrator-facing, and it describes any breaking changes.
  • All new external dependencies have been approved and mentioned in LICENSE.md (if any).
  • All human-readable, translateable strings in this PR been introduced using t() (if any).
  • I manually verified the change (e.g., in manual tests or documentation).
  • The target branch is correct.

martnpaneq
martnpaneq previously approved these changes Apr 2, 2026
Copy link
Copy Markdown
Contributor

@martnpaneq martnpaneq left a comment

Choose a reason for hiding this comment

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

I think that using .port file is not the best solution, as it could be confusing when using two repositories. On the other hand I don't see any better solution.

@martnpaneq martnpaneq dismissed their stale review April 2, 2026 11:01

After some local testing, we keep two instances of the .port file, depending on the repository where the script was invoked. It is too confusing, and I think we should go with reading the port number from the cli instead.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

@przemyslaw-zan przemyslaw-zan changed the title Reading manual tests port from file. Use dynamic port assignment for manual test CI. Apr 2, 2026
Copy link
Copy Markdown
Contributor

@martnpaneq martnpaneq left a comment

Choose a reason for hiding this comment

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

The changes look much better after implementing the new approach.

@przemyslaw-zan przemyslaw-zan merged commit 5e2913a into master Apr 2, 2026
12 checks passed
@przemyslaw-zan przemyslaw-zan deleted the ci/4352 branch April 2, 2026 14:07
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