Conversation
…r all tests closes #189
|
We still need documentation here, but let's get the code in first. |
jcundill
left a comment
There was a problem hiding this comment.
I'll have a closer look tomorrow - but initial thoughts are.
-
Wow, it is a lot faster 👍
-
I had my
cypress.jsonfile set up to just recognise .feature files as test files. This causes cypress itself to ignore any .features file you add and it keeps insisting that it can't find any specs to run. Took ages to track this down. Changed thecypress.jsonto
{
"testFiles": "**/*.*(feature|features)"
}and it was OK again.
-
File watching seems broken - I'll need to take a closer look at this but it seemed like in cypress open if I run the All.features and then change the contents of .feature files then those changes didn't cause cypress to re-run my scenarios.
-
non global step definitions scoped to a given feature are not working as expected - but I think you guys are already aware of this
devtools shows me something like
require('/Users/jcundill/code/cypress-cucumber-preprocessor-bug-reproduction/cypress/integration/common/index.js')
require('/Users/jcundill/code/cypress-cucumber-preprocessor-bug-reproduction/cypress/integration/common/step.js')
describe(`first`, function() {
createTestsFromFeature('First.feature', `Feature: first
Scenario: 1
Given I open page
Scenario: 2
Given I open page
`);
})
describe(`Second`, function() {
createTestsFromFeature('Second.feature', `Feature: Second
Scenario: 1
Given I open page
And I change the steps
Scenario: 2
When It all goes wrong
Given I open page
`);
})
At least some of those requires would need to be inside the describe blocks.
This should be fixable in the featureLoader by working out a set of requires common to all scenarios and a map of feature file name to requires specific to that feature file and requiring the locally scoped files within the describe blocks of just those files that want them.
global step definitions seem to work fine on this PR as they don't suffer from this problem.
As mentioned, will give a more detailed code level review tomorrow
|
Great summary @jcundill 4 - I'll have a tweak as per your suggestion in a moment. Just cleaning it up. About 3 (watching) - I don't think this is a high priority right now, the "all" mode should really be used more for CI, than everyday development, so I'm fine with that limitation, as long as the watcher we have still works, and I don't see why that would change |
lib/featuresLoader.js
Outdated
| .find(fileSteps => fileSteps[filePath]) | ||
| [filePath].join("\n")} | ||
|
|
||
| createTestsFromFeature('${filePath}', \`${spec}\`); |
There was a problem hiding this comment.
think this would need to be
createTestsFromFeature('${path.basename(filePath)}', \${spec}`);
`
to recreate the way cucumber.json files are currently generated - passing in the full path causes issues on windows boxes due to the c: part.
There was a problem hiding this comment.
hmmm ok! That leaves a possibility of feature names collisions (which I'd assumed was not a problem when every feature ran separately), but maybe that's not something to worry about prematurely
lib/getStepDefinitionsPaths.js
Outdated
| filePath | ||
| )}/**/*.+(js|ts)`; | ||
|
|
||
| console.log("GOZDECKI nonGlobalPattern", nonGlobalPattern); |
There was a problem hiding this comment.
Probably need to remove this line
…ch the original loader behavior
|
I've created a PR to update the example repository and as you can see, I had to change one step definition for it to work due to feature names collisions. |
|
🎉 This PR is included in version 1.15.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
@laistomazz very nice! I think what needs to happen now also is you bumping the dependency to the 1.15.0 , then I can merge |
|
@lgandecki done ;) |
this is a #217 PR with a bit of tweaking. The feature is described in #189