Skip to content
This repository was archived by the owner on May 10, 2021. It is now read-only.

Fix cypress:local: Skip test for cache control header #118

Merged
merged 1 commit into from
Dec 19, 2020

Conversation

FinnWoelm
Copy link
Contributor

This fixes a failing test when running npm run cypress:local.

The failing test is testing the cache control header of a pre-rendered, static page and expecting the headers to include "public". This is the case on netlify.app. However, in netlify dev this cache control header is not present, causing the test to fail.

For now, we simply skip the test when running the Cypress tests locally (as we do in default_spec.js). In the long term, we should probably set headers in a _headers file as suggested by #110.

The cache control header in netlify dev does not include "public" for
static assets. This is unlike netlify.app. We should probably set
headers in a _headers file as suggested by:
#110

For now, we simply skip the test when running the Cypress tests locally.
});
if (Cypress.env("DEPLOY") !== "local") {
cy.request("/subfolder/static").then((response) => {
expect(response.headers["cache-control"]).to.include("public");
Copy link
Contributor

Choose a reason for hiding this comment

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

lowercase cache-control? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should it be uppercase? 😅 We have it lowercase in default_spec.js, too, and it seems to be working:

expect(response.headers["cache-control"]).to.include("private");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently, headers are supposed to be case-insensitive: cypress-io/cypress#2879 (comment)

Cypress transforms all headers to lowercase, so that devs don't have to worry about it: cypress-io/cypress#2879 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

dope!! merge away!!!!!!

@lindsaylevine lindsaylevine added bug type: bug code to address defects in shipped code and removed bug labels Dec 15, 2020
@FinnWoelm FinnWoelm merged commit 5ffce26 into main Dec 19, 2020
@FinnWoelm FinnWoelm deleted the fix-cypress-local-cache-control branch December 19, 2020 02:37
lindsaylevine added a commit that referenced this pull request Jan 3, 2021
- Support for i18n in Next 10 ([#75](#75))
- dependabot: node-notifier from 8.0.0 to 8.0.1 ([#125](#125))
- Expose Netlify function params as netlifyFunctionParams ([#119](#119))
- Fix: local cypress cache control ([#118](#118))
- Fix: add res.finished to createResponseObject ([#117](#117))
- Fix: Windows support ([#101](#101))
- Improve logs for specified functions/publish dirs ([#100](#100))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants