Skip to content
New issue

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

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

Already on GitHub? # to your account

[#564] Tighten docs around force404 behaviour #876

Merged
merged 1 commit into from
Aug 29, 2018
Merged

[#564] Tighten docs around force404 behaviour #876

merged 1 commit into from
Aug 29, 2018

Conversation

richdouglasevans
Copy link
Contributor

@richdouglasevans richdouglasevans commented Aug 29, 2018

closes #564

This change isn't as trivial as I thought originally 'cos there are a few places in the docs that have "incorrect" info. I'm scare quoting there because I'm now doubting my own understanding of the behaviour in light of the docs: I'm writing some (more) tests to prove the behaviour.

I verified the behaviour locally: routes that are not matched will, by default, just pass through to the server.

screen shot 2018-08-29 at 10 38 54

---

Start a server to begin routing responses to `cy.route()` and `cy.request()`.
Start a server to begin routing responses to {% url `cy.route()` route %} and {% url `cy.request()` request %}.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hyperlinking the first mention of these other commands in order to be helpful.

@@ -67,7 +65,7 @@ Option | Default | Description

***After starting a server:***

- Any request that does not match a {% url `cy.route()` route %} will be sent a `404` status code.
- Any request that does **NOT** match a {% url `cy.route()` route %} will {% url 'pass through to the server' network-requests#Don’t-Stub-Responses %}.
Copy link
Contributor Author

@richdouglasevans richdouglasevans Aug 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The meat of this PR:

  • document the correct behaviour
  • hyperlink to the relevant section of the Network Requests page to be helpful.

@@ -53,7 +51,7 @@ Option | Default | Description
Option | Default | Description
--- | --- | ---
`enable` | `true` | pass `false` to disable existing route stubs
`force404` | `false` | forcibly send XHR's a 404 status when the XHR's do not match any existing
`force404` | `false` | forcibly send XHR's a 404 status when the XHR's do not match any existing route
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the sentence ended abruptly so I added a trailing route to make it read smoother.

@richdouglasevans richdouglasevans changed the title [#564] Correct mention of force404 behavior [WIP] [#564] Correct mention of force404 behavior Aug 29, 2018
2. The `POST /messages` will match our 2nd route and respond with a `200` status code with the message object.
3. The `GET /updates` did not match any routes and its response automatically sent back a `404` status code with an empty response body.
You can {% url 'read more about this behavior here.' server#Options %}
{% endnote %}
Copy link
Contributor Author

@richdouglasevans richdouglasevans Aug 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so this is rather a large edit, possibly the largest I've made so far. Do steer me Jennifer.

I've removed this example in favour of a short block that directs the reader to the documentation about force404 on the server page. It's a good example but it essentially duplicates a much shorter example on the server page. I've attempted to preserve the thrust of this section while also directing the reader to the server page because the force404 option is applied at the level of the server rather than the route.

Yeah? 🤔

screen shot 2018-08-29 at 12 51 06

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine and a good use of a 'note' as it is outside of the flow of the content.


```javascript
cy.server({force404: true})
cy.server({ force404: true })
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I'm directing the reader to the "canonical" documentation about the force404 option on the server page.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we have lost the info about the headers being null - I haven't verified this is what truly does happen though, so I'm trusting that it was documented correctly to begin with. 😅

@@ -103,9 +101,9 @@ Adding delay can help simulate real world network latency. Normally stubbed resp
cy.server({delay: 1500})
```

***Prevent sending 404's to unmatched requests***
***Send 404s on unmatched requests***
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This header logic was the wrong way round: the force404 option doesn't prevent; rather, it enables the sending of a 404 on unmatched requests so I've changed the header to reflect that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@richdouglasevans richdouglasevans changed the title [WIP] [#564] Correct mention of force404 behavior [#564] Correct mention of force404 behavior Aug 29, 2018
@richdouglasevans richdouglasevans changed the title [#564] Correct mention of force404 behavior [#564] Tighten docs around force404 behaviour Aug 29, 2018
@jennifer-shehane jennifer-shehane self-requested a review August 29, 2018 15:18

```javascript
cy.server({force404: true})
cy.server({ force404: true })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we have lost the info about the headers being null - I haven't verified this is what truly does happen though, so I'm trusting that it was documented correctly to begin with. 😅

2. The `POST /messages` will match our 2nd route and respond with a `200` status code with the message object.
3. The `GET /updates` did not match any routes and its response automatically sent back a `404` status code with an empty response body.
You can {% url 'read more about this behavior here.' server#Options %}
{% endnote %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine and a good use of a 'note' as it is outside of the flow of the content.

@@ -103,9 +101,9 @@ Adding delay can help simulate real world network latency. Normally stubbed resp
cy.server({delay: 1500})
```

***Prevent sending 404's to unmatched requests***
***Send 404s on unmatched requests***
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jennifer-shehane jennifer-shehane merged commit 8bf0179 into cypress-io:develop Aug 29, 2018
@richdouglasevans richdouglasevans deleted the issue/564-server-force404 branch August 29, 2018 16:46
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unclear about Server doc?
2 participants