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

Missing assert_stderr and similar assertions #42

Open
jbriales opened this issue Dec 12, 2021 · 10 comments
Open

Missing assert_stderr and similar assertions #42

jbriales opened this issue Dec 12, 2021 · 10 comments

Comments

@jbriales
Copy link

The run command in bats-core has the ability to keep stderr-related output into separate variables:

Use --separate-stderr to split stdout and stderr into $output/$stderr and ${lines[@]}/${stderr_lines[@]}

as per https://bats-core.readthedocs.io/en/latest/writing-tests.html#run-test-other-commands

Should we have assertion helpers that apply to stderr only?

@martin-schulze-vireso
Copy link
Member

martin-schulze-vireso commented Dec 12, 2021

Since this is a relatively recent addition to bats-core, I would be cautious to add a feature that relies on a recent addition. We would need some kind of guard to avoid breaking suites by using the wrong bats version. I also dislike the amount of code duplication that entails. Would prefixing assert_output with output="$stderr" be a bad option?

@jbriales
Copy link
Author

I think the proposed workaround is very spot on and indeed works nicely and unblocks here.
I think with this alternative, it's not worth cluttering the code and duplicating functionalities as you mentioned.
Closing this. Thanks!

@viathor
Copy link

viathor commented Oct 20, 2022

@jbriales @martin-schulze-vireso I'd like to revisit this.

I agree there is a viable workaround. However, normally bats tests tend to look very clean and elegant and the workaround is
rather jarring. Also, --separate-stderr is no longer as recent as it was back when this was first filed.

Code duplication could be avoided by implementing this as a flag, say -r, --stderr added to both assert_output and assert_line.

WDYT?

@jbriales
Copy link
Author

I agree on bats being generally quite clean and this workaround getting dealing with stderr checks less seamless or "native".

I'm not sure about the impl details, but if we went for extending, I'd find having assert_stderr more discoverable than assert_output --stderr.

@martin-schulze-vireso
Copy link
Member

I agree on bats being generally quite clean and this workaround getting dealing with stderr checks less seamless or "native".

I'm not sure about the impl details, but if we went for extending, I'd find having assert_stderr more discoverable than assert_output --stderr.

I'd Vote for this as well. The existing flag parser in assert_output Just assumes flags it does not know to be the test expectation. This means you will get a somewhat misleading error message in older versions.

@viathor
Copy link

viathor commented Oct 20, 2022

Yeah, assert_stderr sounds good, too. I suppose the counterpart for assert_line would be something like assert_stderr_line or assert_line_stderr?

@jbriales
Copy link
Author

jbriales commented Oct 23, 2022

Yeah, assert_stderr sounds good, too. I suppose the counterpart for assert_line would be something like assert_stderr_line or assert_line_stderr?

Because it would refer to a check in a specific line vs checking against the full output, I'd say assert_stderr and assert_stderr_line would be the most intuitive pair?

@martin-schulze-vireso
Copy link
Member

Because it would refer to a check in a specific line vs checking against the full output, I'd say assert_stderr and assert_stderr_line would be the most intuitive pair?

assert_stderr and assert_stderr_line sounds good to me.

@viathor
Copy link

viathor commented Oct 23, 2022

Sounds like there is agreement that this should be done (and even how it should be done). Perhaps it could be reopened? :-)

debarshiray added a commit to debarshiray/toolbox that referenced this issue Nov 10, 2022
This needs a directory that's going to be present on the host operating
system across various configurations of all supported distributions,
such as the hosts running the CI, but not inside the Toolbx containers.

It looks like /etc/kernel is present on both Debian and Fedora, but
absent from the fedora-toolbox images.  On a Debian 10 server, it's
owned by several packages:
  $ dpkg-query --search /etc/kernel
  dkms, systemd, grub2-common, initramfs-tools, apt: /etc/kernel

... while on Fedora 36 Workstation:
  $ rpm --file --query /etc/kernel
  systemd-udev-250.8-1.fc36.x86_64

Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the lines
array.

[1] bats-core/bats-assert#42

containers#1153
debarshiray added a commit to debarshiray/toolbox that referenced this issue Nov 10, 2022
This needs a directory that's going to be present on the host operating
system across various configurations of all supported distributions,
such as the hosts running the CI, but not inside the Toolbx containers.

It looks like /etc/kernel is present on both Debian and Fedora, but
absent from the fedora-toolbox images.  On a Debian 10 server, it's
owned by several packages:
  $ dpkg-query --search /etc/kernel
  dkms, systemd, grub2-common, initramfs-tools, apt: /etc/kernel

... while on Fedora 36 Workstation:
  $ rpm --file --query /etc/kernel
  systemd-udev-250.8-1.fc36.x86_64

Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42

containers#1153
debarshiray added a commit to debarshiray/toolbox that referenced this issue Nov 16, 2022
Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42
debarshiray added a commit to debarshiray/toolbox that referenced this issue Nov 16, 2022
Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42

containers#1160
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 9, 2022
Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42

containers#1195
debarshiray added a commit to debarshiray/toolbox that referenced this issue Dec 12, 2022
Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42

containers#1195
debarshiray added a commit to debarshiray/toolbox that referenced this issue Oct 11, 2023
Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42

containers#1386
debarshiray added a commit to debarshiray/toolbox that referenced this issue Oct 12, 2023
Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42

containers#1386
debarshiray added a commit to debarshiray/toolbox that referenced this issue Oct 12, 2023
Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42

containers#1386
@hgl
Copy link

hgl commented Oct 14, 2023

Another case against output="$stderr" is that if you use shellcheck, it's going to complain stderr not being defined, so you'd have to either use ${stderr?} or add a shellcheck disable comment.

With assert_stderr, there is no need to jump through such hoops.

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 a pull request may close this issue.

4 participants