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

proposal: testing: verify failure (rather than skip) #28488

Closed
lavalamp opened this issue Oct 30, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@lavalamp
Copy link

commented Oct 30, 2018

I want to write many tests. Because I run CI, I want them all to pass all the time. Because I have not implemented all of my features, some of them actually fail: I want to skip them. Because over time I want to fix my features, I want to be notified when formerly failing tests begin passing so I can stop skipping them.

There is no good way to do this with the go testing package. I think this is a much better thing to do than just skip as long as the failures are deterministic.

My first thought is to do something like in #16502:

// ShouldFail is meant to be used as: `defer ShouldFail(t)`; it allows the
// test to still be run even when we expect it to fail.
func ShouldFail(t *testing.T) {
        if t.Failed() {
                t.Skip("this test is known to currently be broken")
        } else {
                t.Error("expected a failure, but test exited successfully")
        }
}

This doesn't work, because Skip doesn't un-fail a test, which is reasonable.

My second thought was to write a wrapper for *testing.T which overrides implementation for Error, Fatal, etc methods. This is not good because it's viral: I can't pass the type into functions that were expecting *testing.T as a parameter, because testing.T is type struct.

I can think of a number of ways to solve the problem if I can change the testing.T type:

  • Add a "CancelFailure()" method that just clears the failure bit.
  • Add a top-level "ShouldFail()" method instead of making people write it.

I can think of more invasive changes, too, but they're not worth mentioning due to the cost of change.

@gopherbot gopherbot added this to the Proposal milestone Oct 30, 2018

@gopherbot gopherbot added the Proposal label Oct 30, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Oct 30, 2018

Dup of #25951?

@lavalamp

This comment has been minimized.

Copy link
Author

commented Oct 30, 2018

Definitely related, but I'd be very unhappy with a command-line-flag based solution? It's hard to tell what the actual proposal is there.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Oct 30, 2018

If this is the same problem as #25951, then let's close it in favor of that one, because that one is on hold for testing v2.

@lavalamp

This comment has been minimized.

Copy link
Author

commented Oct 30, 2018

I guess that means it'll be considered for v2? If there's no chance of changing v1.x then I guess that's fine.

@lavalamp lavalamp closed this Oct 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.