Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.Sign up
--require not being honoured with --dry-run #1324
Cucumber does not honour the -r flag, and Cucumber provides no way load *.env.rb files during dry runs. From what I understand, cucumber ignores env files due to interactions with Rails. Why this option is not configurable is a mystery as not every Ruby project is a Rails project.
The --require option has the following help text: (emphasis mine)
I am not sure how much more explicit I can be. Sure I could add another rb file and require my env file, but that brings up 2 more problems:
Given I start cucumber is started with the following:
Cucumber does not honour the -r option:
Cucumber should honour the -r option.
Steps to Reproduce (for bugs)
Context & Motivation
I just want to do a dry run so I can find unused steps. I don't see why Rails specific behaviour should affect non-Rails usage.
In general the loading of files by cucumber is not very well defined. We know that files in support folders are loaded first, but beyond that it is a mystery. I have had issues with tests randomly starting to fail because cucumber decided to load xyz.rb before env.rb. I can't require env.rb because of #1043
@dpsi Can I get a few more details on your directory structure? Anything under
Here is an example: https://github.com/dpsi/cucumber-test
So is cucumber's behaviour wrong or is the help text wrong? As you can see, -r is not honoured during dry runs.
In the first one, the output is as expected, env.rb was loaded, and it put "You should see this when env.rb is loaded" to the screen.
@danascheider First, thanks for verifying this on your end.
The env.rb would be the one requiring libraries, setting constants, among other things. All in all, it sets up the environment. Without it, other files will end up with uninitialized constants, no method errors, etc.
While doing googling for my issue, I came across this: https://groups.google.com/d/msg/cukes/Kucz1T_VBss/VkEUBpY6MQAJ
So the dry run surprises have been affecting people for a while now. I'm not sure if there is a willingness to change in time for cucumber 4, but waiting till cucumber 5 isn't practical either.
Is there any other way to change this behaviour other than monkey-patching configuration.rb? Can the help text be changed to reflect the actual behaviour, or can we make --require actually load things?
As a side note: If cucumber decides to ditch this dry run behaviour, you could still maintain the existing behaviour with the --exclude param.
I don't need them at all, but cucumber still has to load any non-env files, and any code not inside a step or method would still get executed, even during a dry-run. So if env,rb sets @config then when my_setup_helper.rb tries to access @config[:some_var] it will get a no method error.
I understand that perhaps my issue is unusual, but I am working on a fairly old codebase (we just upgraded from cucumber 2.4 to 3.1 last month, and we have been using cucumber since at least 1.3) so there are some instances where we might be doing something that is a code smell.
Cleaning up the .rb files is in the works, but removing unused step defs is a bigger priority since they take up majority of the execution time during cucumber's startup. After monkey patching cucumber to work around the env problem, it looks like I have 239 unused step defs spread out over dozens of files. You can maybe grasp the size of repo I am working on.
Not sure if you have already past feature cut for cucumber 4, but adding env files into dry runs along with a note about using exclude to maintain previous behaviour seems like a small change. If the only logic around loading is only controlled by that one file you mentioned, then the change would be 1 line of code.
I definitely understand the challenges of working with a legacy code base. It would definitely be a small change, but I would need to get the input of other maintainers before I could make such a change. In the Google group link you shared, @mattwynne appears to suggest he'd be open to this kind of change in the API but I'd also be curious what @aslakhellesoy has to say about it, especially since we try to keep changes consistent across all Cucumber implementations.
As you've recognised, @dpsi, the current behaviour around requiring files is kinda crufty and incoherent. I'd love this to get some attention for 5.0 if you were interested in helping us spec it out in a more holistic / comprehensive manner.
This was referenced
Nov 7, 2018
Sorry I’m late to join the discussion. When we added dry runs back in the day, I think the purpose was to validate that all the feature files are syntactically correct. Nothing more.
Validating that feature files are correct only requires parsing. No user code needs to be loaded or executed.
I’m curious to understand what additional behaviour you are expecting from a dry run. Or put differently: To you - what is the purpose of a dry run?
I see there is a PR in flight, but let’s not rush into solutions just yet.
I’m reopening this issue as it’s not resolved yet. We’ll close it when a fix is merged to master, or if the suggestion is rejected altogether.
My position is that no files should be loaded during a dry run, and that
@aslakhellesoy I'm trying to both validate feature files, and to find un-used step-defs. I think the issue is that Cucumber uses Kernel#load, which causes code to be executed. Even if we instead require the file, code would still be executed. dry-run is somewhat misleading, when it loads all the files it never calls any of the step definitions you wrote. Therefore it is a 'dry-run' in the sense that Cucumber never actually runs through the Scenarios. The caveat is that Ruby by design interprets and executes code immediately if it's not guarded inside a class, method, or module. This is comparable to what happens when you source a file in a shell such as bash.
I just realised (by reading the changelog) that the purpose of dry run is also to print information about what step definitions are implemented/undefined.
I was a bit surprised about this (I never use this feature myself), but I can see how it can be useful.
With this insight I think it makes sense to load exactly the same files as during a “wet” run. The
The only effect should be that step definitions are not executed.
If this is what the PR does I’m happy with it!