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.

Sign up for GitHub

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

Jump to bottom

parallel: drop --keep-order to stream test output with -j #826

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

parallel: drop --keep-order to stream test output with -j #826

wants to merge 1 commit into from

Conversation

cyphar
Copy link
Contributor

@cyphar cyphar commented Dec 12, 2023

The Tap 14 specification explicitly allows for out-of-order Test Points, which means we do not need to use --keep-order. The primary benefit of doing this is that users will get more feedback during long test runs as to whether tests are passing or not.

Test Points may be output in any order, but any Test Point ID provided
must be within the range described by the Plan.

Technically, Tap 13 didn't disallow out-of-order Test Points (and implies they are allowed) but at the time of implementing "bats -j" support this wasn't clear enough to justify the risk of producing spec-invalid output.

Fixes: da65d1b ("bats: add support for parallel job execution with GNU parallel")
Signed-off-by: Aleksa Sarai cyphar@cyphar.com

@cyphar cyphar requested a review from a team as a code owner December 12, 2023 02:36
@cyphar cyphar force-pushed the parallel-stream-output branch from 624b534 to 3b24035 Compare December 12, 2023 02:38
@cyphar
Copy link
Contributor Author

cyphar commented Dec 12, 2023

Hmmm, this isn't sufficient. bats_forward_output_for_parallel_tests and all of that infrastructure is designed to output things in-order...

@martin-schulze-vireso
Copy link
Member

Even if we we're only using parallel, without --keep-order wouldn't we get interleaved test output?

@cyphar
Copy link
Contributor Author

cyphar commented Dec 14, 2023 •
edited
Loading

GNU Parallel doesn't interleave output by default (the management process waits for the job to finish then outputs the collected stdout and stderr in one go). You can enable interleaving with --line-buffer, but --group is the default behaviour. The reason I used --keep-order in the original implementation was purely because of the (perceived) requirement to have in-order test point output.

I can add --group as an explicit option if you'd prefer that.

@martin-schulze-vireso
Copy link
Member

Well then you still dont get intermediate Output, Theo only improvement would be to not wait on slower earlier tests.

My plan is to replace parallel with the bats internal bash semaphores, which would allow custom handling of the output.

@cyphar
Copy link
Contributor Author

cyphar commented Dec 14, 2023

The issue is that currently bats -j doesn't print anything until the entire test suite has finished. The goal of this PR is to make it so that (like bats without -j) you get output when each individual test has finished.

My plan is to replace parallel with the bats internal bash semaphores, which would allow custom handling of the output.

If the plan is to repurpose the file-level semaphore stuff for the whole suite, fixing the file-level stuff to not wait for tests to finish in order is still necessary.

@cyphar
Copy link
Contributor Author

cyphar commented Dec 15, 2023 •
edited
Loading

Oh, now I see what you meant by:

Well then you still dont get intermediate Output, Theo only improvement would be to not wait on slower earlier tests.

Because even single-file test runs are run as a "suite" and we use GNU Parallel for suite runs, you won't get intermediate output because the whole file run needs to finish for GNU Parallel to output anything. That is really unfortunate...

Yeah, I think the only "nice" solution would be to migrate the parallel logic in bats-exec-suite) to be done in bash with semaphores and removing the collation of output on the suite level. We would need to have a mutex for stdio to make sure the output doesn't get interleaved, but aside from that it shouldn't change much.

There are some other possible workarounds (prepending all lines from bats-exec-file with the name of the job, and then collating them in bats-exec-suite so we can use line-based interleaving with parallel --line-buffer), but I'm not sure how much sense they make if the plan is to remove the GNU Parallel dependency.

In any case, I'll push a version that allows for un-collated bats-exec-file output.

@cyphar
parallel: drop --keep-order to stream test output with -j
eeaa352 
The Tap 14 specification explicitly allows for out-of-order Test Points,
which means we do not need to use --keep-order. The primary benefit of
doing this is that users will get more feedback during long test runs as
to whether tests are passing or not.

> Test Points *may* be output in any order, but any Test Point ID provided
> must be within the range described by the Plan.

Technically, Tap 13 didn't disallow out-of-order Test Points (and implies
they are allowed) but at the time of implementing "bats -j" support this
wasn't clear enough to justify the risk of producing spec-invalid output.

Fixes: da65d1b ("bats: add support for parallel job execution with GNU parallel")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
@cyphar cyphar force-pushed the parallel-stream-output branch from 3b24035 to eeaa352 Compare December 15, 2023 01:15
@cyphar cyphar marked this pull request as draft December 15, 2023 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cyphar @martin-schulze-vireso