Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(network-shim): only incrementally update missing request stub state #209

Merged
merged 2 commits into from
May 27, 2021

Conversation

HendrikThePendric
Copy link
Contributor

@HendrikThePendric HendrikThePendric commented May 27, 2021

CLI-48 turns out to be a regression from yesterday #201. It has to do with the fact that state changes during a test in a stub run (it’s always done that) and state now also being persisted across tests in a stub run (it used to do this in capture mode only).

I considered two options for addressing this issue:

  1. The current solution which keeps using the state to keep track of missing request stubs across features
  2. A more involved, but arguably cleaner, solution which would involve keeping track of missing request stubs during stub mode via a dedicated state object.

I opted for 1 because this doesn't require as many changes, and hope the comments compensate somewhat for the complicated logic.

@HendrikThePendric HendrikThePendric self-assigned this May 27, 2021
@mediremi mediremi self-requested a review May 27, 2021 15:17
Co-authored-by: Médi-Rémi Hashim <[email protected]>
@HendrikThePendric HendrikThePendric merged commit e2ccea8 into alpha May 27, 2021
@HendrikThePendric HendrikThePendric deleted the CLI-48-fix-malformed-json-error branch May 27, 2021 15:31
dhis2-bot added a commit that referenced this pull request May 27, 2021
# [8.0.0-alpha.4](v8.0.0-alpha.3...v8.0.0-alpha.4) (2021-05-27)

### Bug Fixes

* **network-shim:** only incrementally update missing request stub state ([#209](#209)) ([e2ccea8](e2ccea8))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 8.0.0-alpha.4 🎉

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 <[email protected]>
@dhis2-bot
Copy link
Contributor

🎉 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants