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

New design or logic for test_subprocess.py #181

Open
ccarouge opened this issue Oct 12, 2023 · 0 comments · May be fixed by #253
Open

New design or logic for test_subprocess.py #181

ccarouge opened this issue Oct 12, 2023 · 0 comments · May be fixed by #253
Assignees
Labels
priority:low Low priority Issues that do not need to be addressed in the near future. testing Add unit/integration tests

Comments

@ccarouge
Copy link
Collaborator

A lot of the tests under TestRunCmd in test_subprocess.py are greedy: they test more than they say they are testing.

  • test_stdout_is_suppressed_in_non_verbose_mode is supposed to test stdout but it carries assert clauses for stdout and stderr.
  • test_stderr_is_suppressed_in_non_verbose_mode is also testing on stdout and stderr.
  • test_command_and_stdout_is_printed_in_verbose_mode and test_command_and_stderr_is_redirected_to_stdout_in_verbose_mode are identical tests with different inputs and outputs. Should we parametrize?
  • test_output_is_captured_with_capture_output_enabled and test_stderr_captured_to_stdout are the same with different input and outputs. But should the first test only test on stdout and not stderr?
  • test_command_is_printed_and_stdout_is_captured_in_verbose_mode: why test on stderr?
  • test_stdout_is_redirected_to_file: why test anything else but the file content?
  • test_command_is_printed_and_stdout_is_redirected_to_file_in_verbose_mode: why test on stderr?

We could also redesign the tests to be for a series of cases using parameterisation (I'm not sure all are necessary). Tests would be:

  • test stdout only
  • test stderr only
  • test captured stdout (capture_output=True) only
  • test captured stderr only
  • test file printing only

And run all of these on this series of inputs:

  • command only
  • command + verbose
  • command + capture_output
  • command + capture_output + verbose
  • command + output_file
  • command + output_file + verbose
  • same as all the above but with redirection of the command to stderr (echo foo 1>&2)

I'm not sure if all these inputs are necessary or if we need more (do we need to test for capture_output + file printing?).

Also, rename captured in the tests as that is confusing with the capture_output option.

@abhaasgoyal abhaasgoyal self-assigned this Jan 31, 2024
@abhaasgoyal abhaasgoyal linked a pull request Feb 15, 2024 that will close this issue
@SeanBryan51 SeanBryan51 added the priority:medium Medium priority issues to become high priority issues after a release. label Mar 21, 2024
@ccarouge ccarouge added priority:low Low priority Issues that do not need to be addressed in the near future. and removed priority:medium Medium priority issues to become high priority issues after a release. labels Apr 3, 2024
@SeanBryan51 SeanBryan51 added the testing Add unit/integration tests label Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:low Low priority Issues that do not need to be addressed in the near future. testing Add unit/integration tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants