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

Report fork spec failures #1412

Merged
merged 2 commits into from
Mar 18, 2021
Merged

Report fork spec failures #1412

merged 2 commits into from
Mar 18, 2021

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Mar 17, 2021

When a test running under our expect_in_fork test helper fails, it creates this output:

  1) Datadog::Workers::AsyncTraceWriter integration tests forking when the process forks and a trace is written with :async fork policy does not drop any traces
     Failure/Error: expect(status && status.success?).to be true
     
       expected true
            got false
     # ./spec/support/synchronization_helpers.rb:13:in `expect_in_fork'
     # ./spec/ddtrace/workers/trace_writer_spec.rb:688:in `block (6 levels) in <top (required)>'
     # ./spec/spec_helper.rb:164:in `block (2 levels) in <top (required)>'(required)>'

This message unfortunately only tells us that the child process exited with a status non-zero.
This error happens to be a flaky issue today: https://app.circleci.com/pipelines/github/DataDog/dd-trace-rb/2998/workflows/5c01a51a-b03d-4bfb-a32d-5015716207df/jobs/120042/parallel-runs/2

This PR captures the STDERR output inside the forked process and reports that output when the fork exists with a non-zero status. I introduced a fake failure (expect(1).to be(2)) at line trace_writer_spec.rb:689 for this example, inside the forked block:

  0) Datadog::Workers::AsyncTraceWriter integration tests forking when the process forks and a trace is written with :async fork policy does not drop any traces
     Failure/Error: expect(status && status.success?).to be(true), stderr

       expected #<Integer:5> => 2
            got #<Integer:3> => 1

       	from ./spec/ddtrace/workers/trace_writer_spec.rb:689:in `block (7 levels) in <top (required)>'
       	from ./spec/support/synchronization_helpers.rb:12:in `block in expect_in_fork'
       	from ./spec/support/synchronization_helpers.rb:8:in `fork'
       	from ./spec/support/synchronization_helpers.rb:8:in `expect_in_fork'
       	from ./spec/ddtrace/workers/trace_writer_spec.rb:688:in `block (6 levels) in <top (required)>'
       	...
       	from /Users/marco.costa/.rbenv/versions/2.7.1/bin/rspec:23:in `<main>'
     # ./spec/support/synchronization_helpers.rb:24:in `expect_in_fork'
     # ./spec/ddtrace/workers/trace_writer_spec.rb:688:in `block (6 levels) in <top (required)>'
     # ./spec/spec_helper.rb:164:in `block (2 levels) in <top (required)>'

@marcotc marcotc added the dev/testing Involves testing processes (e.g. RSpec) label Mar 17, 2021
@marcotc marcotc self-assigned this Mar 17, 2021
@marcotc marcotc requested a review from a team March 17, 2021 16:15
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@marcotc marcotc merged commit 37c2879 into master Mar 18, 2021
@marcotc marcotc deleted the report-fork-error branch March 18, 2021 19:54
@github-actions github-actions bot added this to the 0.47.0 milestone Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev/testing Involves testing processes (e.g. RSpec)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants