Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

BugFix: Random test failure when tracking compilation time #1713

Merged
merged 2 commits into from
Jan 31, 2025

Conversation

CodeMeister
Copy link
Contributor

Summary

When running the full test suite, it would randomly fail an activesupport_instrumentation test. On re-running the suite several times, everything would pass.

Test:

  • "tracks proper time of compiling the factory"

Source:

  • acceptance/activesupport_instrumentation_spec.rb:99

The Problem:

it "tracks proper time of compiling the factory" do
    time_to_execute = 0
    callback = ->(_name, start, finish, _id, _payload) { time_to_execute = finish - start }
    ActiveSupport::Notifications.subscribed(callback, "factory_bot.compile_factory") do
      FactoryBot.build(:user)
    end

    expect(time_to_execute).to be > 0
  end

The test would fail roughly once in every 30 runs , always with a :time_to_execute of 0:

The Cause

The cause, was that "factory_bot.compile_factory" instrumentation triggered twice, not just once.

  • First for :user
  • Second for :configuration

The :configuration compilation time, which can indeed be 0, is the last one triggered, so that's the elapsed time recorded by :time_to_execute.

The Solution

By changing :time_to_execute to a hash, and recording the individual compilations, we can check just the :user compilation time with time_to_execute[:user].

it "tracks proper time of compiling the factory" do
  time_to_execute = {user: 0}
  callback = ->(_name, start, finish, _id, _payload) {
    time_to_execute[_payload[:name]] = (finish - start)
  }
  ActiveSupport::Notifications.subscribed(callback, "factory_bot.compile_factory") do
    FactoryBot.build(:user)
  end

  expect(time_to_execute[:user]).to be > 0
end

test: "tracks proper time of compiling the factory"
source: acceptance/activesupport_instrumentation_spec.rb:99

The test sporadically fails with a complitation time of 0:

The issue, is that the "factory_bot.compile_factory"
intrumentation is actually triggered twice, not just once.

First for :user and secondly for :configuration.

The :configuration compilation time, which can indeed be 0,
is the last one triggered, so that's the elapsed time recorded
by :time_to_execute.

By changing :time_to_execute to a hash, and recording the individual
compilations, we can check just the :user compilation time with
`time_to_execute[:user]`.
@CodeMeister CodeMeister changed the title BugFix for random test failure when tracking compilation time BugFix: Random test failure when tracking compilation time Nov 5, 2024
Copy link
Member

@neilvcarvalho neilvcarvalho left a comment

Choose a reason for hiding this comment

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

Thank you for fixing it! I could see this test failure, but didn't dig deep into it. I added a minimal change that I can commit myself if you'd like.

spec/acceptance/activesupport_instrumentation_spec.rb Outdated Show resolved Hide resolved
@CodeMeister
Copy link
Contributor Author

@neilvcarvalho happy for you to change it

@neilvcarvalho neilvcarvalho merged commit 9bf88a5 into thoughtbot:main Jan 31, 2025
25 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants