Skip to content

Add back actioncable rails6 specs#1277

Merged
ericmustin merged 4 commits into
masterfrom
fix_action_cable_specs
Dec 8, 2020
Merged

Add back actioncable rails6 specs#1277
ericmustin merged 4 commits into
masterfrom
fix_action_cable_specs

Conversation

@ericmustin

Copy link
Copy Markdown
Contributor

Previously we'd disabled action_cabled rails6 specs as they were hanging in CI. I can't recreate this locally so trying to trigger in CI again, we've also done some work on the tracer as well as test suite since these were originally disabled that may have resolved the issue (but i doubt it)

#951

@ericmustin ericmustin requested a review from a team December 7, 2020 16:18
@ericmustin ericmustin changed the title [wip] Add back actioncable rails6 specs Add back actioncable rails6 specs Dec 7, 2020
@ericmustin

Copy link
Copy Markdown
Contributor Author

@marcotc i've run this a few times on ci and many times locally without being able to reproduce the original issue, i wonder if work related to either tracer shutdown or running the ci tests in parallel have resolve the flakiness? I've added back logging here in case some errors were getting swallowed when this does fail.

@marcotc marcotc added the dev/ci Involves CircleCI, GitHub Actions, or GitLab label Dec 8, 2020
@ericmustin ericmustin merged commit b19ed71 into master Dec 8, 2020
@marcotc marcotc added this to the 0.44.0 milestone Dec 8, 2020
ivoanjo added a commit that referenced this pull request Feb 17, 2021
The newly-released
[sucker_punch](https://github.com/brandonhilkert/sucker_punch) 3.0.0
gem includes a few changes to support Ruby 3.0 keyword syntax
(see brandonhilkert/sucker_punch#235), but it
made our current tests fail in JRuby.

Relevant snippet is (from
[this failed run](https://app.circleci.com/pipelines/github/DataDog/dd-trace-rb/2764/workflows/596c0f11-c7dd-4e89-a36a-78397cc724c6/jobs/112458/parallel-runs/21)):

```
F.E, [2021-02-17T08:12:15.473545 #1277] ERROR -- : Sucker Punch job error for class: '#<Class:0x17abef0f>' args: [:fail, {}]
ArgumentError wrong number of arguments (given 2, expected 0..1)
/app/spec/ddtrace/contrib/sucker_punch/patcher_spec.rb:62:in `perform'
/usr/local/bundle/jruby/2.5.0/gems/sucker_punch-3.0.0/lib/sucker_punch/job.rb:62:in `__run_perform'
/app/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:32:in `block in __run_perform'
/app/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:77:in `block in __with_instrumentation'
/app/lib/ddtrace/tracer.rb:284:in `trace'
/app/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:73:in `__with_instrumentation'
/app/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:21:in `__run_perform'
/usr/local/bundle/jruby/2.5.0/gems/sucker_punch-3.0.0/lib/sucker_punch/job.rb:38:in `block in perform_async'
/usr/local/bundle/jruby/2.5.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/java_executor_service.rb:79:in `run'

Failures:

  1) sucker_punch instrumentation failed job should instrument a failed job
     Failure/Error: expect(job_span).to have_error_type('ZeroDivisionError')
       expected Span with error type "ArgumentError" to have error type "ZeroDivisionError"
     # ./spec/ddtrace/contrib/sucker_punch/patcher_spec.rb:100:in `block in <main>'
     # ./spec/ddtrace/contrib/sucker_punch/patcher_spec.rb:23:in `block in <main>'
     # /usr/local/bundle/jruby/2.5.0/gems/webmock-3.11.2/lib/webmock/rspec.rb:37:in `block in <main>'
```

An unexpected `ArgumentError` is occuring in JRuby, making our testcase
fail (because it was actually trying to cause an exception to see if it
was properly traced).

This issue is caused by a bug in sucker_punch's delegation which then
triggered a JRuby/TruffleRuby bug. I've reported the bug to the
appropriate upstreams (see #1369), and this workaround makes our CI
happy again so that we don't need to wait for upstream to react.

Fixes #1369
@ivoanjo ivoanjo deleted the fix_action_cable_specs branch July 16, 2021 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev/ci Involves CircleCI, GitHub Actions, or GitLab

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants