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

Sort tests #34

Closed
wants to merge 2 commits into from
Closed

Sort tests #34

wants to merge 2 commits into from

Conversation

mvidner
Copy link
Member

@mvidner mvidner commented Jul 22, 2016

@mvidner
Copy link
Member Author

mvidner commented Jul 22, 2016

An alternative is to use a random but deterministic order, with rspec --order random ... which will print out the random seed used and allow reproducing the problem.

But I think such randomization should only be used on a noncritical build path, similarly to what we do with -Werror for C/++ packages.

@mvidner
Copy link
Member Author

mvidner commented Jul 22, 2016

An example of a fix of order-dependent test bug is yast/yast-network#421

@ancorgs
Copy link
Contributor

ancorgs commented Jul 22, 2016

Should we really do this or let the tests fail and fix them one by one? They are not supposed to be order-dependent.

@ancorgs
Copy link
Contributor

ancorgs commented Jul 22, 2016

In other words, I would vote for the alternative proposed in @mvidner's comment.

@mvidner
Copy link
Member Author

mvidner commented Jul 22, 2016

Now I realize that it should have been debuggable even before, because rake prints out how it calls rspec.

The main problem with the randomized tests is that they tend to break at inopportune moments, and it will put the team in a situation where you are fixing a bug that a colleague made but you can't even tell them "c'mon you should've ran the tests" because they did. And the benefit is IMHO small.

@jreidinger
Copy link
Member

@mvidner I disagree, as order dependent tests also break when you work locally with given test. And when bootloader failed, I also rerun test with exact order written in travis and see if it can be reproduced. so also 👎 for this change.

@jreidinger
Copy link
Member

@mvidner @ancorgs so as it is already written out how exactly it is ran, I think alternative is already implemented, so are you OK with closing this PR?

@mvidner
Copy link
Member Author

mvidner commented Sep 19, 2016

I still disagree and think this PR is valid, but let's close it now and argue when/if the problem appears again.

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.

3 participants