cucumber-ruby icon indicating copy to clipboard operation
cucumber-ruby copied to clipboard

Namespaced worlds cannot access each other

Open mitchgrout opened this issue 2 years ago • 3 comments

👓 What did you see?

Constructed a pair of namespaced worlds, in which one internally used the other to manage some common resources. Both worlds were observed to be defined and non-nil within test steps. Within each world instance however, the other world were observed to be defined, but nil.

✅ What did you expect to see?

Expected either:

  • neither world to have the other defined as a local
  • both worlds to have the other defined, and set to the correct global instance

📦 Which tool/library version are you using?

  • ruby-cucumber, v8.0.0.rc.1
  • ruby, v3.0.0
  • Ubuntu 20.04 LTS, running kernel 5.4.0

🔬 How could we reproduce it?

  1. Create the following feature file
Feature: Namespaced worlds can access each other
  Scenario: World A can access the instance of World B
    Given the test step sees world B as non nil
    Then the instance of world A sees the instance of world B the same as the test step sees world B

  Scenario: World B can access the instance of World A
    Given the test step sees world A as non nil
    Then the instance of world B sees the instance of world A the same as the test step sees world A
  1. Create the following step definition
module WorldA
  def observe_b
    world_b
  end
end

module WorldB
  def observe_a
    world_a
  end
end

World(world_a: WorldA, world_b: WorldB)

Given /^the test step sees world A as non nil$/ do
  expect(world_a).to_not be nil
end 

Given /^the test step sees world B as non nil$/ do
  expect(world_b).to_not be nil
end

Then /^the instance of world A sees the instance of world B the same as the test step sees world B$/ do
  expect(world_a.observe_b).to be world_b
end

Then /^the instance of world B sees the instance of world A the same as the test step sees world A$/ do
  expect(world_b.observe_a).to be world_a
end
  1. Run the tests, and observe the following results:
$ cucumber --quiet
Feature: Namespaced worlds can access each other

  Scenario: World A can access the instance of World B
    Given the test step sees world B as non nil
    Then the instance of world A sees the instance of world B the same as the test step sees world B
      
      expected #<Object:49460> => #<Object:0x0000557415f3f640>
           got #<NilClass:8> => nil
      
      Compared using equal?, which compares object identity,
      but expected and actual are not the same object. Use
      `expect(actual).to eq(expected)` if you don't care about
      object identity in this example.
      
       (RSpec::Expectations::ExpectationNotMetError)
      ./features/step_definitions/namespaced_world.rb:24:in `/^the instance of world A sees the instance of world B the same as the test step sees world B$/'
      features/namespaced_world.feature:4:in `the instance of world A sees the instance of world B the same as the test step sees world B'

  Scenario: World B can access the instance of World A
    Given the test step sees world A as non nil
    Then the instance of world B sees the instance of world A the same as the test step sees world A
      
      expected #<Object:49480> => #<Object:0x0000557416096ea8>
           got #<NilClass:8> => nil
      
      Compared using equal?, which compares object identity,
      but expected and actual are not the same object. Use
      `expect(actual).to eq(expected)` if you don't care about
      object identity in this example.
      
       (RSpec::Expectations::ExpectationNotMetError)
      ./features/step_definitions/namespaced_world.rb:28:in `/^the instance of world B sees the instance of world A the same as the test step sees world A$/'
      features/namespaced_world.feature:8:in `the instance of world B sees the instance of world A the same as the test step sees world A'

Failing Scenarios:
cucumber features/namespaced_world.feature:2
cucumber features/namespaced_world.feature:6

2 scenarios (2 failed)
4 steps (2 failed, 2 passed)

📚 Any additional context?

This issue arose while looking at factoring out common behaviours from some namespaced worlds to one consolidated spot. The intent was to have all of the other worlds access the common world to get access to the shared behaviour. This appeared to be a valid refactoring, since with non-namespaced worlds, each world is able to access the other methods introduced by the others.

There does not appear to be any documentation indicating that this should or should not be possible.


This text was originally generated from a template, then edited by hand. You can modify the template here.

mitchgrout avatar Apr 21 '22 09:04 mitchgrout

Hi there. Perfect repro scenario and perfect documentation.

I guess you've asked the million dollar question. Should you be able to do this.

I've worked on cucumber for years, and rarely if ever use namespaced worlds. As usually my world is reasonably thin / I use ruby namespacing. So my "gut" says we shouldn't allow world transversal. But this is only my opinion.... We definitely need more eyes on this first.

EDIT: You mentioned they were objects but nil objects. The fact the methods do return an object suggests partial code paths. Imo these should probably error.

luke-hill avatar Apr 21 '22 10:04 luke-hill

Looking back at this, I think my answer to the question would be no. Ultimately the point of a namespaced world is to provide some logical organisation of test state, which should remain roughly independent from other state. As such, the idea of some shared world which others can access feels a bit antithetical.

I would support a patch to ensure that a world simply cannot observe any others, meaning that in a setup like...

module Foo
  def observe_bar
    bar
  end
end

module Bar
end

World(foo: Foo, bar: Bar)

then foo.observe_bar should raise NameError rather than the current behaviour of returning nil. This would emphasise that the worlds are intended to be fully isolated from each other.

mitchgrout avatar May 14 '22 07:05 mitchgrout

Ideally it should probably raise an NME as it should have no context if completely isolated. But yes, glad we're on the same page.

Next step, find some time to work on this. I can definitely pick this up part time in the coming month, but other than that no idea how tricky it is.

luke-hill avatar May 17 '22 07:05 luke-hill