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

Add option to print failing group #34

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mRudzki
Copy link
Contributor

@mRudzki mRudzki commented May 12, 2023

Why this option is useful?
If you stumble across random failures you can debug it easier.

@mRudzki mRudzki force-pushed the Add-option-to-print-failing-group branch from 4042794 to 152e694 Compare May 12, 2023 10:24
Copy link
Collaborator

@ilyazub ilyazub left a comment

Choose a reason for hiding this comment

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

Thanks, @mRudzki 👍

Why the regular failure RSpec output is not enough?

@mRudzki
Copy link
Contributor Author

mRudzki commented May 29, 2023

I did it separately from #33 but maybe I shouldn't .
Many projects I worked on have some old/poorly written specs that fail only on specific occasion.

Example 1

it "one" do
  record = build :record
  expect { record.save }.to eq(true)
end

it "two" do
  record = build :record, id: 1
  expect { record.save }.to eq(true)
end

In this simple example, order "two, one" will succeed but "one, two" will fail.

Example 2

A more real world (I had that exact problem on some old spec last month) example is setting thread variables and forgetting to unset them later.

# file1.rb
let(:client) { create :client }
...
it "saves currrent_user_id in execution_context_store" do
  expect { complex_class.new(user: client).call }.to change(Thread.current[:execution_context_store]).from(nil).to({client: client})
end

# file2.rb
  after do
    Thread.current[:execution_context_store] = {}
  end

  describe ".store" do
    before do
      Thread.current[:execution_context_store] = { some: "value" }
    end

    it "returns value stored in current thread 'execution_context_store'" do
      expect(described_class.store).to eq(some: "value")
    end
  end

If file1.rb will be on same thread will be before file2.rb and won't be cleared with "after" we will have an error.

Why would it be beneficial?

Printing failing group, prints tests that were in that group. If we combine it with seed option, we can do bundle exec rspec --bisect
and get the failing and specs it is depending on.
Without it we have to run the whole suite, and even if we'll know failing seed on turbo_tests we won't be able to bisect on it.
Here is why:

.
├── spec
│   ├── services
│   │   ├── file1.rb
│   │   ├── file2.rb
│   │   ├── file3.rb
│   │   ├── file4.rb
└── ...

Let's assume we have an example 2
but we also have:

# file3.rb
  after do
    Thread.current[:execution_context_store] = {}
  end
 ...

Let's assume turbo_tests seed 4321 -n 2 will return order: Thread1: "file1.rb, file2.rb", Thread2: "file3.rb, file4.rb"
Result fail
Let's assume "normal" rspec seed 4321 will return order: "file1.rb, file3.rb, file2.rb, file4.rb"
Result: no fail because file3.rb prevented failure

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