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 specs files to be consistent #1537

Merged
merged 1 commit into from
Jul 22, 2016

Conversation

ggrossetie
Copy link
Member

Currently tests are sharing a global state @env.
Sorting specs files is important to make errors reproducible by everyone.

Ref #1536

@iliabylich
Copy link
Contributor

Sorting specs files is important to make errors reproducible by everyone

But it also hides possible errors. Currently when you run specs in the alphabet order you get no errors. But when you run it in the reverse order you get an error in rescue_spec.

I thought it's a good practice to run specs in the random order.

MSpec has on option called randomize that shuffles files, I think it makes sense to enable it to catch possible conflicts between specs.

@ggrossetie
Copy link
Member Author

You are right but I don't like random failures because it's pretty hard to reproduce and make the build unstable.
I would rather prefer a consistent behavior for every developers.

Catching bugs between specs is nice but specs are mainly designed to catch bugs in the "production" code not in the "testing" code 🐛

@iliabylich
Copy link
Contributor

👍

  • If we want to run a test suite in a normal (i.e. sequential) order, we simply run it as is. The order can't depend on the platform or locale(?), this is wrong.
  • If we want to a random order, we add MSpec.randomize(true) to the spec_helper by setting some environment variable.

@elia Any thoughts?

@@ -58,14 +58,14 @@ module MSpecSuite

# Filters must be added first
suite_filters = suite == 'opal' ? opalspec_filters : rubyspec_filters
add_specs["#{suite} filters", suite_filters]
add_specs["#{suite} filters", suite_filters.sort]
Copy link
Contributor

Choose a reason for hiding this comment

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

By the way, there's no need to sort spec filters, they can't conflict

@elia
Copy link
Member

elia commented Jul 21, 2016

Ideally we should add a shuffle after seeding srand with a number we also print out, that should make the random order reproducible, but still random.

> srand 123; (1..10).to_a.shuffle
=> [5, 1, 8, 6, 9, 4, 2, 7, 10, 3]
> srand 123; (1..10).to_a.shuffle
=> [5, 1, 8, 6, 9, 4, 2, 7, 10, 3]
> srand 123; (1..10).to_a.shuffle
=> [5, 1, 8, 6, 9, 4, 2, 7, 10, 3]

@ggrossetie
Copy link
Member Author

Ideally we should add a shuffle after seeding srand with a number we also print out, that should make the random order reproducible, but still random.

Yes that's a good idea! Reproducible is the key 😄

@elia
Copy link
Member

elia commented Jul 22, 2016

Yes that's a good idea! Reproducible is the key 😄

To be fair, it's RSpec team's idea💡 😄

@iliabylich
Copy link
Contributor

@elia It can be solved after #1540. Can we merge this PR for now?

@elia
Copy link
Member

elia commented Jul 22, 2016

sure thing

@elia elia merged commit eed9ceb into opal:master Jul 22, 2016
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.

None yet

3 participants