Skip to content

Fix flakey time-sensitive notes creation specs that rely on background Sidekiq jobs

Eric Eastwood requested to merge 48093-fix-flakey-notes-background-process-specs into master

Fix flakey time-sensitive notes creation specs that rely on background Sidekiq jobs

Found while working on https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/6051, failing test: https://gitlab.com/gitlab-org/gitlab-ee/-/jobs/74649298

Here are some details on the different Sidekiq testing modes (thanks @stanhu for this important debugging breakthrough),

  • Sidekiq::Testing.fake!/Sidekiq::Testing.fake?: Pushes jobs into an jobs array and need to be manually drained, NewNoteWorker.drain or Sidekiq::Worker.drain_all
  • Sidekiq::Testing.inline!/Sidekiq::Testing.inline?: Runs jobs synchronously

We rely on background jobs like the cross-references being setup for notes in some tests but currently we set Sidekiq::Testing.fake! in Spec::Support::Helpers::Features::NotesHelpers -> add_note("Hello world!") and we should expect this test to fail in all cases because the background job is never run/drained.

The only reason it is working now is because of a race-condition on when exactly a Sidekiq NewNoteWorker job is queued. Currently, it passes because the job is queued outside of the Sidekiq::Testing.fake! block and runs synchronously,

See the Sidekiq source __set_test_mode()

# Normal Passing
$ SELENIUM_REMOTE_URL=http://localhost:4444/wd/hub bundle exec spring rspec spec/features/issuables/markdown_references/internal_references_spec.rb:65
SIDEKIQ __set_test_mode fake
SIDEKIQ __set_test_mode->begin fake
SIDEKIQ __set_test_mode->ensure inline
# When the job is queued
SIDEKIQ raw_push Sidekiq::Testing.fake?=false Sidekiq::Testing.inline?=true

# Proxy failing
$ SELENIUM_REMOTE_URL=http://localhost:4545/wd/hub bundle exec spring rspec spec/features/issuables/markdown_references/internal_references_spec.rb:65
SIDEKIQ __set_test_mode fake
SIDEKIQ __set_test_mode->begin fake
# When the job is queued
SIDEKIQ raw_push Sidekiq::Testing.fake?=true Sidekiq::Testing.inline?=false
SIDEKIQ __set_test_mode->ensure inline

Slack discussion here: https://gitlab.slack.com/archives/C02PF508L/p1528963806000272

-- https://gitlab.com/gitlab-org/gitlab-ce/issues/48093

Other debuging notes

  • spec/features/issuables/markdown_references/internal_references_spec.rb
  • spec/support/helpers/features/notes_helpers.rb
  • app/workers/new_note_worker.rb
  • app/services/notes/create_service.rb
  • create_cross_references! -> app/models/concerns/mentionable.rb

Some Sidekiq puts logging,

puts "test-before2 Sidekiq testing modes:\t\t\t\tenabled?#{Sidekiq::Testing.enabled?} disabled?#{Sidekiq::Testing.disabled?} fake?#{Sidekiq::Testing.fake?} inline?#{Sidekiq::Testing.inline?}"
stats = Sidekiq::Stats.new
puts "stats.processed=#{stats.processed} stats.failed=#{stats.failed} stats.enqueued=#{stats.enqueued} stats.queues['new_note']=#{stats.queues['new_note']}"
puts "Sidekiq::Queues #{Sidekiq::Queues.to_json}"

workers = Sidekiq::Workers.new
puts "workers #{workers.size} #{workers.to_json}"

ds = Sidekiq::DeadSet.new
puts "dead #{ds.size}"

rs = Sidekiq::RetrySet.new
puts "rs #{rs.size}"

Find Sidekiq gem source

$ bundle show Sidekiq
# See `__set_test_modeq` and `raw_push` in `lib/sidekiq/testing.rb`

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/48093

Edited by Eric Eastwood

Merge request reports