Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 22, 2025

  • Parallelize test execution with background jobs
  • Use perl buffering to prevent output interleaving
  • Track test failures with flag file
  • Fix port conflicts between tests
    • test_auto_netcat.sh uses ports 12345, 12346, 12347, 12348, 12349 (incrementing)
    • test_auto_injection_netcat.sh uses port 12355 (to avoid overlap with netcat)
    • test_auto_background_job.sh uses port 8080
    • test_auto_injection_node.sh uses port 8081 (to avoid overlap with background_job)
  • Fix race condition in test_auto_netcat.sh TEST 0 - added sleep between server start and client connect
  • Add perl output buffering (perl -e '$| = 1; print while <>;') to ensure test output is flushed immediately and not interleaved
Original prompt

This section details on the original issue you should resolve

<issue_title>Speed up test by running in parallel</issue_title>
<issue_description>pipe output to different files, wait for jobs, and print output. make sure all tests using ports needing servers use test server</issue_description>

<agent_instructions>the run_tests script should run the tests in parallel. to do that, they need to pipe the output into several files, adn then print them sequentially upon completion - we cannot have interleaved output. also, as soon as one tesst fails, we need to make sure we still fail, and only exit with zero exit code if all tests pass.

also, sometimes tests use hardcoded ports, we need to makre sure, no test uses the same hardcoded port, otherwise these tests may conflict. in such a case, we probably should adjust so that they dont use different ports</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Enhance test execution by running in parallel Parallelize test execution to reduce runtime Nov 22, 2025
Copilot AI requested a review from plengauer November 22, 2025 14:16
@plengauer plengauer self-requested a review November 22, 2025 16:52
Copy link
Owner

@plengauer plengauer left a comment

Choose a reason for hiding this comment

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

@copilot this changeset to the run_tests script is too big. make only minimal changes and dont use fancy things like bash arrays, they are scary. i suggest just creating background jobs and waiting for them and making sure all these jobs use separate files to do their stuff. i suggest taking the original loop and transforming the body into a subshell started in background and we wait at the end. we can "buffer" the output to the main script with the perl trick

Copy link
Contributor Author

Copilot AI commented Nov 22, 2025

@copilot this changeset to the run_tests script is too big. make only minimal changes and dont use fancy things like bash arrays, they are scary. i suggest just creating background jobs and waiting fo...

Simplified the implementation per your feedback. Changes made in commits bf3ad48, 12b7d4a, and e1af25a:

  • Removed bash arrays
  • Wrapped the original loop body in a subshell ( ... ) & for background execution
  • Used process substitution < <(...) instead of pipe to allow wait to work properly
  • Added failure flag file for tracking test failures
  • Used perl buffering to prevent output interleaving

The approach is now minimal: just background the test execution, use perl for buffering, and wait at the end.

@plengauer plengauer marked this pull request as ready for review November 22, 2025 17:41
@plengauer plengauer enabled auto-merge (squash) November 22, 2025 17:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR parallelizes test execution in run_tests.sh to significantly reduce CI/test cycle duration. Tests previously ran sequentially; now they execute concurrently while maintaining clean output and proper failure tracking.

Key changes:

  • Background job execution with process substitution for parallel test runs
  • Perl-based output buffering to prevent interleaved console output
  • Temporary flag file mechanism to track and propagate test failures
  • Port conflict resolution between test_auto_netcat.sh (port 12345) and test_auto_injection_netcat.sh (now port 12346)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
tests/run_tests.sh Implements parallel test execution with background jobs, output buffering via perl, and failure tracking using a temporary flag file
tests/auto/test_auto_injection_netcat.sh Changes hardcoded port from 12345 to 12346 to prevent conflicts with test_auto_netcat.sh when tests run concurrently

moflwi
moflwi previously approved these changes Nov 23, 2025
@plengauer plengauer marked this pull request as ready for review November 27, 2025 22:04
Copilot AI review requested due to automatic review settings November 27, 2025 22:04
@plengauer plengauer enabled auto-merge (squash) November 27, 2025 22:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.

@plengauer plengauer requested a review from moflwi November 27, 2025 22:56
@plengauer plengauer self-requested a review November 28, 2025 18:06
Copy link
Owner

@plengauer plengauer left a comment

Choose a reason for hiding this comment

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

@copilot there are tests failing, pls fix

auto-merge was automatically disabled November 28, 2025 18:15

Head branch was pushed to by a user without write access

Copy link
Contributor Author

Copilot AI commented Nov 28, 2025

@copilot there are tests failing, pls fix

Fixed in df1b118. Found another port conflict: test_auto_injection_node.sh and test_auto_background_job.sh both used port 8080. Changed test_auto_injection_node.sh to use port 8081 instead.

Copilot AI requested a review from plengauer November 28, 2025 18:20
@plengauer plengauer enabled auto-merge (squash) November 28, 2025 18:56
@plengauer
Copy link
Owner

@copilot there is an error in the test run, please solve: https://github.com/plengauer/Thoth/actions/runs/19771228924/job/56658212272

auto-merge was automatically disabled November 29, 2025 11:13

Head branch was pushed to by a user without write access

Copy link
Contributor Author

Copilot AI commented Nov 29, 2025

@copilot there is an error in the test run, please solve: https://github.com/plengauer/Thoth/actions/runs/19771228924/job/56658212272

Fixed in 4bf343b. The test_auto_netcat.sh TEST 0 was failing due to a race condition - the client was connecting before the listening server was ready. Added a 1-second sleep between starting the server and connecting.

@plengauer
Copy link
Owner

Copy link
Contributor Author

Copilot AI commented Nov 29, 2025

@copilot also kindly fix this error: https://github.com/plengauer/Thoth/actions/runs/19771228924/job/56658212314

Fixed in 76500d8. Added perl output buffering (perl -e '$| = 1; print while <>;') to the test execution pipeline to ensure test output is properly flushed and not lost when tests run in parallel. This should fix the missing test output and ensure all SUCCEEDED/FAILED messages are captured.

@plengauer plengauer marked this pull request as draft November 30, 2025 08:43
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.

Speed up test by running in parallel

3 participants