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

roachtest: remove stable vs unstable distinction #32796

Merged
merged 1 commit into from
Dec 3, 2018

Conversation

petermattis
Copy link
Collaborator

Remove the distinction between stable vs unstable tests. That
distinction had more value in the past when it was useful to delineate
which tests were stable and which weren't and when there were more
failures due to the test infrastructure. Now that most tests are stable
and the roachtest infrastruture is mature, the value has
lessened. Rather than keeping the burden of managing the "stable" tag,
we're just removing it.

Release note: None

Remove the distinction between stable vs unstable tests. That
distinction had more value in the past when it was useful to delineate
which tests were stable and which weren't and when there were more
failures due to the test infrastructure. Now that most tests are stable
and the roachtest infrastruture is mature, the value has
lessened. Rather than keeping the burden of managing the "stable" tag,
we're just removing it.

Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@petermattis
Copy link
Collaborator Author

bors r=andreimatei

craig bot pushed a commit that referenced this pull request Dec 3, 2018
32796: roachtest: remove stable vs unstable distinction r=andreimatei a=petermattis

Remove the distinction between stable vs unstable tests. That
distinction had more value in the past when it was useful to delineate
which tests were stable and which weren't and when there were more
failures due to the test infrastructure. Now that most tests are stable
and the roachtest infrastruture is mature, the value has
lessened. Rather than keeping the burden of managing the "stable" tag,
we're just removing it.

Release note: None

Co-authored-by: Peter Mattis <petermattis@gmail.com>
@craig
Copy link
Contributor

craig bot commented Dec 3, 2018

Build succeeded

@craig craig bot merged commit db416c0 into cockroachdb:master Dec 3, 2018
@petermattis petermattis deleted the pmattis/roachtest-stable branch December 4, 2018 14:10
@danhhz
Copy link
Contributor

danhhz commented Dec 4, 2018

I'm 👎 on this. In both partitioning and then changefeeds, it was quite useful to have an unstable test that would give us a daily datapoint but was ignored by anyone investigating the roachtest failures for a release. If the motivation is the burden of maintaining the tag, perhaps we could switch the default to stable and only specify it for the unstable tests.

@tbg
Copy link
Member

tbg commented Dec 4, 2018

@danhhz as a counterpoint, I only look at issues and issues get posted for both. Do we also want to stop issues from being posted (hopefully not)? Perhaps you can get a similar result by appending [unstable] to a failed test's issue and communicate the intent that way, without all the boilerplate of maintaining the abstraction officially. (You could also put "unstable" in the test name, but that seems like the kind of thing that would never be changed).

@petermattis
Copy link
Collaborator Author

As one of the frequent investigators of roachtest failures, it isn't onerous to remember that a particular test was recently added and thus unstable.

@tbg
Copy link
Member

tbg commented Dec 4, 2018

I think @danhhz might be worried about whoever is on release rotation (and thus isn't necessarily experienced). But I think noting the instability in the issue for the test is going to work out OK.

@danhhz
Copy link
Contributor

danhhz commented Dec 4, 2018

The issues are useful as a convenient history, so I wouldn't want those to go away.

But I think noting the instability in the issue for the test is going to work out OK.

I think it's quite optimistic to think the release engineer is going to read enough of the issue to notice this. OTOH, the worst case here is an extra round of questions from individuals that don't frequently investigate roachtest failures. And it sounds like I've been the main (only?) person having this issue. I think the burden of maintaining this feature is being overstated here, but perhaps a test naming convention is the appropriate compromise.

@tbg
Copy link
Member

tbg commented Dec 4, 2018

I think it's quite optimistic to think the release engineer is going to read enough of the issue to notice this

In case that wasn't clear, I was suggesting adding [unstable] to the title (I think the issue poster still finds it, it just searches for the issue it would post). See #30832 for an example. Might not be enough 🤷‍♂️ we will soon find out.

@danhhz
Copy link
Contributor

danhhz commented Dec 4, 2018

In case that wasn't clear, I was suggesting adding [unstable] to the title (I think the issue poster still finds it, it just searches for the issue it would post).

TIL. I don't have a mental model of how this thing works and have been assuming I can't change the issue title at all.

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.

5 participants