fix(network-shim): fix before hook bug #201
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HendrikThePendric
changed the title
HendrikThePendric
force-pushed
the
CLI-46-respect-intercepts-from-before-hook
branch
from
0c78716
to
2482b5e
Compare
HendrikThePendric
force-pushed
the
CLI-46-respect-intercepts-from-before-hook
branch
from
702f13b
to
84e1fab
Compare
|
UPDATE: I tried the current fix in the sms-configuration-app and everything seems to be working correctly. |
Mohammer5
approved these changes
May 27, 2021
packages/cypress-commands/src/setups/enableNetworkShim/stubRequests.js
Outdated
Show resolved
Hide resolved
amcgee
reviewed
May 27, 2021
| if ( | ||
| results.totalFailed > 0 && | ||
| Array.isArray(state.missingRequestStubs) && | ||
| state.missingRequestStubs.length > 1 |
There was a problem hiding this comment.
Might be missing something, but should this be >=1 instead of >1 ?
There was a problem hiding this comment.
Shoot you are right, > 0 would have made most sense. I'll prepare a fix PR and will merge that in cowboy style. Yeehaw 🤠 . (alpha is not protected)
There was a problem hiding this comment.
Could that be related to not being able to reproduce..?
dhis2-bot
added a commit
that referenced
this pull request
May 27, 2021
# [8.0.0-alpha.2](v8.0.0-alpha.1...v8.0.0-alpha.2) (2021-05-27) ### Bug Fixes * **network-shim:** fix before hook bug ([#201](#201)) ([0e1cd4c](0e1cd4c))
|
🎉 This PR is included in version 8.0.0-alpha.2 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
dhis2-bot
added a commit
that referenced
this pull request
Jun 10, 2021
# [8.0.0](v7.0.1...v8.0.0) (2021-06-10) ### Bug Fixes * **network-shim:** ensure DHIS2_BASE_URL is available in localStorage ([#214](#214)) ([741ab4b](741ab4b)) * **network-shim:** ensure in-test fixtures are used instead of shim fixtures ([#176](#176)) ([84a1907](84a1907)) * **network-shim:** fix before hook bug ([#201](#201)) ([0e1cd4c](0e1cd4c)) * **network-shim:** only incrementally update missing request stub state ([#209](#209)) ([e2ccea8](e2ccea8)) * **network-shim:** report missing stubs if at least one is found ([#208](#208)) ([45b3331](45b3331)) * **network-shim:** use electron instead of chrome for runs ([#213](#213)) ([ae73686](ae73686)) ### chore * remove node 10 support ([6245ef2](6245ef2)) ### Code Refactoring * **install command:** combine network shim command & plugin options ([4bc9a4e](4bc9a4e)) * drop the app-start flag ([9674d87](9674d87)) * simplify cypress-plugin and cli-utils-cypress ([dc58462](dc58462)) * wait for baseUrl to become available ([745194f](745194f)) ### Features * **enable auto login:** add option to install command ([e9dde4e](e9dde4e)) * **install cmd:** warn about potentially missing peer depds (temporarily) ([c3046aa](c3046aa)) ### BREAKING CHANGES * **install command:** The two options are merged into one, which is now called "enableNetworkShim". * New minimum version for NodeJS is 12.x. * Drop run and open commands We want to be consistent with how Cypress runs locally and in CI and since we cannot use d2-utils-cypress in CI, we shouldn't run it through d2-utils-cypress locally either. * Change configuration keys to camelCase. - dhis2_username => dhis2Username - dhis2_password => dhis2Password - dhis2_base_url => dhis2BaseUrl - dhis2_datatest_prefix => dhis2DataTestPrefix - dhis2_api_version => dhis2ApiVersion * dhis2_api_stub_mode renamed to networkMode Instead of describing the mode of the plugin, it is a bit easier to understand if we speak in terms of the network: - do we want to capture the network traffic (networkMode=capture), - do we want to stub it (networkMode=stub), - or do we want to run it against a live backend (networkMode=live)? * 'DISABLED' renamed to 'LIVE' To better describe the state of the network when running tests instead of describing the state of the plugin, DISABLED is now LIVE. * isDisabledMode renamed to isLiveMode. Similar to the above, to better describe the state of the network vs. the state of the plugin, replace usages of isDisabledMode with isLiveMode. * 'CAPTURE'|'STUB'|'LIVE' are now lowercase when passed to the environment. Replace networkMode=LIVE with networkMode=live. * Drop the --waitOn flag As of now we wait on the baseUrl that is defined in cypress.json, as that is the URL that the tests are going to run against. * Drop support for the --appStart flag. As a consumer, you are expected to either use something like concurrently to run the app server and the cypress server in a single process, or run then manually in two separate processes. This is no longer done automatically. * **network-shim:** bumps cypress 1 major version, to v7 * fix(network-shim): filter request and response headers properties This was planned anyway to keep fixtures stable. But also turned out to be required due to a bug: cypress-io/cypress#16420 * fix(network-shim): disable auto-login during stub run * fix(network-shim): add 'system/info' resource to static resources list * feat(network-shim): run tests suite on CI * docs(network-shim): add info about the network-shim test suite * chore(network-shim): add command to locally run full e2e suite * docs(network-shim): add info reg troubleshooting and local full test run * chore(cy local): run build command before cypress commands Co-authored-by: Jan-Gerke Salomon <jgs.salomon@gmail.com>
|
🎉 This PR is included in version 8.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes were created because we encountered a problem when trying to implement the NetworkShim in the sms-configuration-app. It seemed that in some cases the network shim intercept callback was still being executed even if there was an intercept with fixture declared in the test itself. And because the NetworkShim was throwing an error the test failed.
I have spent a long time trying to write a reproduction case in the platform-app example. After trying for quite long time and still being unable to reproduce the exact problem I discussed things with @varl. Because the issue could be reproduced consistently in the sms-configuration-app and also solved there fairly easily, and because this PR will get merged to
alphafirst, it was OK to just write a fix without a reproduction case.So at the core of the current fix lies the notion that encountering a missing fixture during a stub run could be a bad thing, but it doesn't have to be a problem since a test could have declared a
cy.intercept()with a fixture or static response object too. So instead of throwing an error I collect the missing fixtures and once the run is done I will show a warning. I only show this warning when:Besides changes related directly to the problem described above, I have also ported the test suite of the example platform-app to use a cypress-cucumber test-suite, so now we have:
I think we can these changes in too, even though they aren't strictly related to the issue at hand.