fix(network-shim): handle 304s from different scenarios in the same feature

[HendrikThePendric]

Stub mode was failing for me in the approval-app. In essence, the problem was that we uniquely identify requests per scenario (within a feature), but the browser doesn't get refreshed between scenarios (it does between features). So the browser was classifying requests as duplicates (304) while we were detecting them as unique....

There would have been two possible ways to tackle this issue:

• We could change our classification scheme to match the browser. I suppose this would have resulted in the cleanest technical solution. But we would have lost one level of granularity in our fixtures because we wouldn't differentiate on scenario-name anymore, only on feature-name. Perhaps that is actually OK, but I worry about unforeseen negative effect because of a higher occurance of duplicates.
• I've opted to continue using both the feature-name and the scenario-name to create a test name. This way we don't change too much in the fundamental logic in the networkshim. If we identify a 304 request stub we try to find a sibling request stub that belongs to the same feature and doesn't have a 304 status code.

This PR contains a lot of changes to generated files. When reviewing only these are relevant:

• These files contain changes to the cypress test suite to reproduce the issue:
  • examples/platform-app/cypress/integration/not_modified.feature
  • examples/platform-app/cypress/integration/common/index.js
  • examples/platform-app/cypress/integration/listing/index.js
• These files contain the actual fix:
  • packages/cypress-commands/src/setups/enableNetworkShim/stubRequests.js
  • packages/cypress-commands/src/setups/enableNetworkShim/utils.js

[amcgee]

@HendrikThePendric won't the chosen second option be hit-or-miss whether tests pass or fail though?

[HendrikThePendric]

@amcgee

won't the chosen second option be hit-or-miss whether tests pass or fail though?

Could you explain why?

You might be right though... I still don't really see why, but I'm trying my fix in the approval app now and am still seeing tests fail.

[HendrikThePendric]

@amcgee I think you're completely right about this. In the approval app my original problem (empty response body) is gone, but now I have a new problem, which is that it's returning the wrong response.

I've put this PR back into draft mode and will have to make some more fundamental changes to the networkShim:

• The property testName will become featureName and not include the scenario name anymore
• All 304s will be handled equally and we will keep leveraging the fact that the requests within a feature will always happen in the exact same sequence

[HendrikThePendric]

@amcgee I've implemented things as I summarised in #261 (comment). This solution makes a lot more sense, thanks for realising the flaw in the original fix in #261 (comment).

[HendrikThePendric]

I've tried this fix in the approval-app and it resolved the issue there.

[HendrikThePendric]

I also considered adding an additional test to verify that this solution is backwards compatible. However setting this up would be quite involved. Instead I have manually tested this in the approval-app:

1. I generated the fixtures by doing a capture run with v8.0.1
2. This generated fixtures with the full name testName containing both the feature and scenario name
3. I then did a stub with this version.
4. Then I installed a build from the current PR and did a stub run

Both stub runs produced exactly the same results. So the solution is backwards compatible.

[dhis2-bot]

🎉 This PR is included in version 8.0.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀