Avoid using fiber local for determining example group. by ioquatix · Pull Request #116 · KnapsackPro/knapsack

ioquatix · 2021-07-13T22:45:49Z

We ran into a problem using Knapsack with Async::RSpec which runs the spec in a nested asynchronous fiber. Because RSpec.current_example is fiber-local, it becomes nil and Knapsack fails.

In addition, it seems like the else side of the branch is incorrect.

ArturT · 2021-07-14T05:54:55Z

Else branch was to support older RSpec version as far as I remember.

ioquatix · 2021-07-28T08:34:05Z

Are you able to merge this?

ArturT · 2021-07-28T09:11:39Z

This PR is on my TODO list. I will get back to it when I'm done with higher priority tasks.

Regarding this PR:

we still need to support older RSpec versions so else branch is needed. Maybe instead of checking current example we could verify if example_group is present in example.metadata[:example_group]. If not then return else branch. But this needs to be tested first.

ArturT · 2021-07-28T09:18:03Z

Here is the old PR that added support for RSpec 2. #1

ioquatix · 2021-07-28T11:10:53Z

@ArturT can you please approve the workflow so it can run CI?

ArturT · 2021-07-28T11:12:50Z

Approved.

ArturT · 2021-07-30T12:38:28Z

Hi @ioquatix

I'm working on PR #117 that will introduce changes you proposed and fix specs and drop support for RSpec 2.x.

I have a separate repository with an example rails app. I'd like to add to it async-rspec gem and an example spec file to verify that the changes in knapsack gem will work. But I can't make the spec spec/async/reactor_spec.rb pass green.

PR with async-rspec gem: https://github.com/KnapsackPro/rails-app-with-knapsack/pull/13/files
I added test based on an example from https://github.com/socketry/async-rspec#async-reactor

I run the test this way:

$ rspec spec/async/reactor_spec.rb
Knapsack time offset warning enabled!

Async::IO
  should send and receive data within the same reactor (FAILED - 1)

========= Knapsack Time Offset Warning ==========
Time offset: 30s
Max allowed node time execution: 35s
Exceeded time: -20s

Global time execution for this CI node is fine.
Happy testing!

Need explanation? See FAQ:
https://docs.knapsackpro.com/ruby/knapsack#faq
=================================================
Read up on the benefits of a dynamic test split with Knapsack Pro Queue Mode:
https://docs.knapsackpro.com/2020/how-to-speed-up-ruby-and-javascript-tests-with-ci-parallelisation

Sign up for Knapsack Pro here:
https://knapsackpro.com
=================================================

Knapsack global time execution for tests: 15s

Failures:

  1) Async::IO should send and receive data within the same reactor
     Got 0 failures and 2 other errors:

     1.1) Failure/Error: output_task.wait

          Async::Stop:
            Async::Stop
          # /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:66:in `yield'
          # /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/condition.rb:40:in `wait'
          # /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:148:in `wait'
          # ./spec/async/reactor_spec.rb:22:in `block (2 levels) in <top (required)>'
          # /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-rspec-1.16.1/lib/async/rspec/reactor.rb:93:in `block (4 levels) in <module:RSpec>'
          # /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-rspec-1.16.1/lib/async/rspec/reactor.rb:57:in `block in run_in_reactor'
          # /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:260:in `block in make_fiber'

     1.2) Failure/Error: raise TimeoutError, "Run time exceeded duration #{duration}s:\n#{buffer.string}"

          Async::TimeoutError:
            Run time exceeded duration 15s:
            #<Async::Reactor:0x4380 1 children (running)>
            	#<Async::Task:0x4394 RSpec::ExampleGroups::AsyncIO (running)>
            	→ /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:62:in `yield'
            	  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:62:in `yield'
            	  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/condition.rb:40:in `wait'
            	  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:148:in `wait'
            	  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-rspec-1.16.1/lib/async/rspec/reactor.rb:67:in `run_in_reactor'
            	  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-rspec-1.16.1/lib/async/rspec/reactor.rb:92:in `block (3 levels) in <module:RSpec>'
            	  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:260:in `block in make_fiber'
            		#<Async::Task:0x43a8 Timer task duration=15. (running)>
            		→ /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:93:in `backtrace'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:93:in `backtrace'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/node.rb:345:in `print_backtrace'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/node.rb:338:in `block in print_hierarchy'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/node.rb:298:in `traverse'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/node.rb:301:in `block in traverse'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/node.rb:88:in `each'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/node.rb:300:in `traverse'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/node.rb:301:in `block in traverse'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/node.rb:88:in `each'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/node.rb:300:in `traverse'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/node.rb:333:in `print_hierarchy'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-rspec-1.16.1/lib/async/rspec/reactor.rb:47:in `block in run_in_reactor'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:260:in `block in make_fiber'
            		#<Async::Task:0x43bc running example (running)>
            		→ /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:62:in `yield'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:62:in `yield'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/condition.rb:40:in `wait'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:148:in `wait'
            		  /Users/artur/Documents/github/knapsack-pro/rails-app-with-knapsack/spec/async/reactor_spec.rb:22:in `block (2 levels) in <top (required)>'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/rspec-core-3.10.1/lib/rspec/core/example.rb:262:in `instance_exec'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/rspec-core-3.10.1/lib/rspec/core/example.rb:262:in `block in run'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/rspec-core-3.10.1/lib/rspec/core/example.rb:508:in `block in with_around_and_singleton_context_hooks'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/rspec-core-3.10.1/lib/rspec/core/example.rb:465:in `block in with_around_example_hooks'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/rspec-core-3.10.1/lib/rspec/core/hooks.rb:486:in `block in run'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/rspec-core-3.10.1/lib/rspec/core/hooks.rb:626:in `block in run_around_example_hooks_for'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/rspec-core-3.10.1/lib/rspec/core/example.rb:350:in `call'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-rspec-1.16.1/lib/async/rspec/reactor.rb:93:in `block (4 levels) in <module:RSpec>'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-rspec-1.16.1/lib/async/rspec/reactor.rb:57:in `block in run_in_reactor'
            		  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:260:in `block in make_fiber'
            			#<Async::Task:0x43d0 (running)>
            			→ /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:62:in `yield'
            			  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:62:in `yield'
            			  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/wrapper.rb:233:in `wait_for'
            			  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/wrapper.rb:124:in `wait_readable'
            			  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-io-1.32.2/lib/async/io/generic.rb:220:in `async_send'
            			  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-io-1.32.2/lib/async/io/generic.rb:62:in `block in wrap_blocking_method'
            			  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-io-1.32.2/lib/async/io/generic.rb:127:in `read'
            			  /Users/artur/Documents/github/knapsack-pro/rails-app-with-knapsack/spec/async/reactor_spec.rb:15:in `block (3 levels) in <top (required)>'
            			  /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:260:in `block in make_fiber'
          # /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-rspec-1.16.1/lib/async/rspec/reactor.rb:50:in `block in run_in_reactor'
          # /Users/artur/.rvm/gems/ruby-3.0.1/gems/async-1.30.1/lib/async/task.rb:260:in `block in make_fiber'

Finished in 15.13 seconds (files took 4.33 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/async/reactor_spec.rb:11 # Async::IO should send and receive data within the same reactor

I tried using higher timeout: 60 like suggested here https://github.com/socketry/async-rspec#changing-timeout but the test is still failing.

Do you have any example test with the async-rspec gem that I could use to verify that changes in knapsack gem will work with async-rspec?

ArturT · 2021-08-03T13:43:57Z

Hi @ioquatix by any chance you have an example spec for async-rspec gem that I could reuse in my test project?

Despite the fact my spec is failing I verified that changes introduced in #117 fix your problem but I'd like to have at least 1 test for async-rspec passing green just in case so we won't introduce regression bug in the future. Thanks.

ArturT · 2021-08-03T18:45:43Z

I managed to make async-rspec test passing by just commenting out its content https://github.com/KnapsackPro/rails-app-with-knapsack/pull/13/files#diff-d0b3815a1363f7e8dce8f34eb23ad8151304b5c4688b47b93ca1956e9217b619R12
This should do for now.

ioquatix · 2021-08-03T21:22:39Z

I'll take a look.

ioquatix · 2021-08-03T21:51:15Z

I think the reason why this fails is because of bugs in 3.0.2 can you please try Ruby 2.7.x? Make sure you use the latest release of async etc.

ArturT · 2021-08-04T10:14:56Z

I've used the latest async, async-io, async-rspec gems.

I tried Ruby versions:

3.0.2
3.0.1
2.7.1
2.6.6

My test spec/async/reactor_spec.rb is still failing.

I'm going to ignore this for now and leave it with a commented-out code in the spec/async/reactor_spec.rb.

ioquatix · 2021-08-04T21:12:51Z

Hmm odd. Can you give me full instructions to reproduce the issue, including what repos to check out and any local gems if needed? Thanks.

ArturT · 2021-08-04T22:07:13Z

You can clone this repo with rails project:
https://github.com/KnapsackPro/rails-app-with-knapsack

Please checkout to branch rspec-current-example-group. It's branch for this pull request.

You can clone knapsack repo as well for this pull request #117 and use this branchrspec-current-example-group.

Local directory structure:

~/Documents/projects <- this is root directory. Put all repos inside of it.
~/Documents/projects/knapsack <- repo for the gem
~/Documents/projects/rails-app-with-knapsack <- rails application here

Inside of the rails-app-with-knapsack repository you can update path for knapsack gem in the Gemfile or set your path as env var KNAPSACK_REPO_PATH before you run bundle install.

When you go to rails-app-with-knapsack repository you can run tests:

rspec spec/async/reactor_spec.rb. Please first uncomment commented out code inside of the test. Then you will see failing test.
If you want to test knapsack gem you can run KNAPSACK_GENERATE_REPORT=true bundle exec rspec spec. This would measure tests time execution and run hooks. This works fine.

Thanks for the help.

ioquatix · 2021-08-05T00:10:00Z

I will take a look this weekend.

ArturT · 2021-08-05T11:20:35Z

I've released knapsack 4.0.0 with the improvent.

You can clone directly master branch for both repositories:

ArturT · 2021-08-05T11:46:11Z

I've released also knapsack_pro 3.0.0 with the same improvement to support async-rspec gem.

Avoid using fiber local for determining example group.

91247a4

ArturT added bug enhancement labels Jul 28, 2021

ArturT mentioned this pull request Jul 30, 2021

Use RSpec example block argument instead of the global RSpec.current_example. This allows to run tests with the async-rspec gem. #117

Merged

ioquatix closed this Aug 3, 2021

ioquatix deleted the patch-1 branch August 3, 2021 21:50

Conversation

ioquatix commented Jul 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArturT commented Jul 14, 2021

Uh oh!

ioquatix commented Jul 28, 2021

Uh oh!

ArturT commented Jul 28, 2021

Uh oh!

ArturT commented Jul 28, 2021

Uh oh!

ioquatix commented Jul 28, 2021

Uh oh!

ArturT commented Jul 28, 2021

Uh oh!

ArturT commented Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArturT commented Aug 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArturT commented Aug 3, 2021

Uh oh!

ioquatix commented Aug 3, 2021

Uh oh!

ioquatix commented Aug 3, 2021

Uh oh!

ArturT commented Aug 4, 2021

Uh oh!

ioquatix commented Aug 4, 2021

Uh oh!

ArturT commented Aug 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ioquatix commented Aug 5, 2021

Uh oh!

ArturT commented Aug 5, 2021

Uh oh!

ArturT commented Aug 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ioquatix commented Jul 13, 2021 •

edited

Loading

ArturT commented Jul 30, 2021 •

edited

Loading

ArturT commented Aug 3, 2021 •

edited

Loading

ArturT commented Aug 4, 2021 •

edited

Loading