Skip to content

Separate stdout and stderr from the run output #2

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

Closed

Conversation

Marthym
Copy link

@Marthym Marthym commented Sep 22, 2017

Put all tested method output in the same variable is an alteration of the real method result.
For exemple :

testedMethod() {
  echo "the result"
  (>&2 echo "some log informations")
  return 0
}
value=$(testedMethod)

value contain only "the result" and when I use it in a script I'm only interested in "the result".
But it's not possible to assert only the stdout. It should be possible to assert separatly stdout and stderr.

That's why I propose this update.
In order to keep compatibility I juste add variables :

  • stdout: Only the stdout output
  • stderr: Only the stderr output
  • stdlines: stdout by line
  • errlines: stderr by line
  • output: Contain stdout+stderr concatenated

@dimo414 dimo414 mentioned this pull request Sep 22, 2017
@Marthym Marthym force-pushed the feature/run-dont-include-stderr branch from d626063 to 38c7b73 Compare September 26, 2017 16:04
@btamayo
Copy link
Member

btamayo commented Sep 28, 2017

Thank you @Marthym! I'll approve this once AppVeyor CI is set up :)

@mbland
Copy link
Contributor

mbland commented Sep 30, 2017

I'm against merging this for now. I see the potential value in separating the streams, but replacing the current way output is calculated with stdout and stderr concatenated would break tests which expect stdout and stderr output to be interleaved.

I'd like to think through and discuss this a little more. I think it's possible to filter stdout from stderr and maintain current output behavior, but it'll require a different approach.

@sublimino
Copy link
Member

I agree with @mbland, but this raises another question: how to detect bats-core changes that break test suites outside of this project (i.e. existing bats users).

Is it worth getting some bats tests from other projects running in CI to avoid break existing contracts with users in ways we don't expect?

Or if not, perhaps we should add a test to core for this condition (interleaved stdout/err).

@mbland
Copy link
Contributor

mbland commented Sep 30, 2017

@sublimino I'll run changes against my mbland/go-script-bash suite at a minimum. ;-)

Ideally, the bats-core suite should suffice to ensure users don't break, since that's what a test suite is for—but it would still be good to build a catalog of exemplar projects to validate, at least before new version releases, that the suite is thorough enough.

@btamayo
Copy link
Member

btamayo commented Sep 30, 2017

@mbland @sublimino I agree with you two. I think changes should follow the semantic versioning strategy, but our bare minimums would be to support bash 3.2 and be TAPS-compatible the minimum.

The ability to separate the different streams would be useful to have, but it needs to pass those two constraints and not change default behavior.

I can update the README and CONTRIBUTING (and possibly tests) to include this information and leave this PR open.

@mbland mbland self-requested a review September 30, 2017 19:21
@Marthym Marthym force-pushed the feature/run-dont-include-stderr branch from 38c7b73 to 3005f28 Compare September 30, 2017 19:53
@Sylvain303
Copy link
Contributor

Hi,

Is it for version 1.0?

I think 1.0 should be able to run old test Bats 0.4.0 natively.
It could be achieved by a switch or a config file, for enabling / disabling this compatibility.

Splitting could be fine. But merging in $output must be explicitly asked by the test.

@mbland
Copy link
Contributor

mbland commented Oct 3, 2017

Yeah, this sounds like v1.0.0 territory per #16. That said, I think we should probably keep the original merged output and offer the split version via an option—or just always offer both, if it's easier to implement and doesn't strain resources and performance.

So to be clear, I think we're a thumbs-up on the feature, but we need to be a bit more deliberate about the implementation.

@mbland mbland added this to the v1.1.0 milestone Oct 9, 2017
@mbland mbland mentioned this pull request Oct 9, 2017
mbland pushed a commit that referenced this pull request Mar 16, 2018
Consistent naming, minor markdown tweaks, add sublimino
@Potherca Potherca mentioned this pull request Apr 25, 2018
@mbland mbland removed the target: 1.0 label Jun 8, 2018
@mbland mbland modified the milestones: v1.0.0, v2.0.0 Jun 8, 2018
@Flamefire
Copy link
Contributor

Would be great to have this in!

@Sylvain303
Copy link
Contributor

Hi,

What about this early feature?

Suggestion: Having $output holding its traditional behavior (merged) and some additional $stdout and $stderr splited values.

My current workaround: (could be logged too of course)

function debug()
{
  if [[ $DEBUG -eq 1 ]] ; then
    # write on non standar non stdout non stderr descriptor
    echo "debug: $*" > /dev/tty
  fi
}

@marshallford
Copy link

I'm also interested in this feature.

@martin-schulze-vireso
Copy link
Member

Since this has aged quite a bit and contains many unrelated changes, I decided to close this but put it on the agenda in #377.

@yarikoptic
Copy link
Contributor

FWIW +1 on having such a feature (can't "vote" on agenda for a specific item so decided to express myself in a comment)

svensson84 added a commit to svensson84/bats-core that referenced this pull request May 13, 2022
martin-schulze-vireso pushed a commit to svensson84/bats-core that referenced this pull request Jul 11, 2022
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.

10 participants