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

Prepend env in command output #884

Merged
merged 1 commit into from
Nov 5, 2022
Merged

Prepend env in command output #884

merged 1 commit into from
Nov 5, 2022

Conversation

adis-io
Copy link
Contributor

@adis-io adis-io commented Oct 17, 2022

This PR combines --verbose-process-command and --verbose-rerun-command into singel --verbose-command.
Also TEST_ENV_NUMBER and PARALLEL_TEST_GROUPS environment variables added to command output. Useful for debugging purposes.

Before:

1 process for 1 spec, ~ 1 spec per process
bundle exec rspec spec/xxx_spec.rb

After:

1 process for 1 spec, ~ 1 spec per process
TEST_ENV_NUMBER= PARALLEL_TEST_GROUPS=2 bundle exec rspec spec/xxx_spec.rb

Checklist

  • Feature branch is up-to-date with master (if not - rebase it).
  • Added tests.
  • Added an entry to the Changelog if the new
    code introduces user-observable changes.
  • Update Readme.md when cli options are changed

@adis-io adis-io marked this pull request as draft October 17, 2022 14:49
@adis-io adis-io changed the title Update runner.rb Prepend env in command output Oct 17, 2022
@adis-io adis-io marked this pull request as ready for review October 24, 2022 18:35
Copy link
Owner

@grosser grosser left a comment

Choose a reason for hiding this comment

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

the only thing we need is the number and the groups right ?
... so can we change it to TEST_ENV_NUMBER=1 FOO=bar xxx
so it's copy-paste friendly

CHANGELOG.md Outdated Show resolved Hide resolved
command = @runner.command_with_seed(command, failing_set[:seed]) if failing_set[:seed]
puts Shellwords.shelljoin(command)
env_str = "TEST_ENV_NUMBER=#{env['TEST_ENV_NUMBER']} PARALLEL_TEST_GROUPS=#{env['PARALLEL_TEST_GROUPS']}"
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
env_str = "TEST_ENV_NUMBER=#{env['TEST_ENV_NUMBER']} PARALLEL_TEST_GROUPS=#{env['PARALLEL_TEST_GROUPS']}"
env_str = ["TEST_ENV_NUMBER", "PARALLEL_TEST_GROUPS"].map { |e| "#{e}=#{env[e]}" }.join(" ")

Copy link
Owner

Choose a reason for hiding this comment

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

... do we even need PARALLEL_TEST_GROUPS ?
I think the reason for this PR was that the re-run did not hit the same db or something like that ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess yes. Will be useful when --only-group is used. Otherwise, it will be difficult to differentiate commands.

@adis-io adis-io force-pushed the patch-1 branch 2 times, most recently from f6283f9 to 9de57ce Compare November 5, 2022 18:32
@grosser grosser merged commit fc09051 into grosser:master Nov 5, 2022
@adis-io adis-io deleted the patch-1 branch November 5, 2022 19:54
@grosser
Copy link
Owner

grosser commented Nov 5, 2022

4.0.0

mcmoyer added a commit to mcmoyer/parallel_tests that referenced this pull request Mar 23, 2023
PR [grosser#884](grosser#884) combined
both `verbose-process-command` and `verbose-rerun-command` into
`verbose-command`  This PR updates the Readme to reflect that change
@mcmoyer mcmoyer mentioned this pull request Mar 23, 2023
4 tasks
grosser pushed a commit that referenced this pull request Mar 23, 2023
PR [#884](#884) combined
both `verbose-process-command` and `verbose-rerun-command` into
`verbose-command`  This PR updates the Readme to reflect that change
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