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

Helper to identify sections of tests #54

Open
wants to merge 2 commits into
base: v1
Choose a base branch
from

Conversation

pascallouisperez
Copy link

Useful for scenarios such as

scenarios := []struct{ ... }{
  scenario1,
  scenario2,
}
for _, s := scenarios {
  c.Section(s.name)
  ...
}

Useful for scenarios such as

  scenarios := []struct{ ... }{
    scenario1,
    scenario2,
  }
  for _, s := scenarios {
    c.Section(s.name)
    ...
  }
@niemeyer
Copy link
Contributor

I don't understand the idea. Why doesn't it simply:

c.Logf("Scenario: %s", s.name)

?

@pascallouisperez
Copy link
Author

Because that will log even if the test passes, making it the output harder to decipher.

@niemeyer
Copy link
Contributor

If that was true your own implementation wouldn't work either, right? You are simply calling c.log in your proposed change.

@pascallouisperez
Copy link
Author

But it's called within logCode, which itself is called from logCaller which itself is conditionally called on test failure. So this line only shows when a test fails.

@pascallouisperez
Copy link
Author

Maybe there's a better place to add this logging, you know the framework better so totally open to suggestion :)

@niemeyer
Copy link
Contributor

Can you please try my suggestion before arguing it doesn't work? :-)

@pascallouisperez
Copy link
Author

I did, hence my PR. Using c.Log or c.Logf will log unconditionally. So if scenario 7 fails, you still have the output for 1 through 6, then the output for 7 and the failure.

@niemeyer
Copy link
Contributor

c.Logf only logs if the test fails.

@niemeyer
Copy link
Contributor

And yes, you will have the history of prior scenarios, but anything is only ever shown if the test fails. If your test outputs "Scenario: foo" last before an assertion fails, that will be the one failing. If you want completely independent logs and test failure/success semantics, I suggest separating the tests.

@pascallouisperez
Copy link
Author

"If you want completely independent logs and test failure/success semantics, I suggest separating the tests."

Having multiple examples in one test is very standard, and separating often leads to repeated code without the benefit of increased readability. This patch provides a nice and simple add on to support it.

Yay or nay?

@niemeyer
Copy link
Contributor

Yes, you have a point.. The way it is done doesn't feel quite right, but let's fins a better way. Let me get back to you on this later today.

@pascallouisperez
Copy link
Author

Ping ;) Have you thought about a better to support this?

@niemeyer
Copy link
Contributor

Yes, sorry for the lack of feedback. I just haven't had a chance to cook a good-looking API yet, but we should have a way to add a some sort of label while iterating over such tables, which is only visible next to an actual failure. Not next to the backtrace, though, but to the error report itself.

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