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

[cookies] Remove cookies when tests complete #12817

Merged
merged 1 commit into from
Sep 4, 2018

Conversation

jugglinmike
Copy link
Contributor

Avoid test interactions by removing cookies at the completion of each
test. Use the add_cleanup feature of testharness.js to ensure that
this occurs regardless of the passing/failing status of each test.
Persist the existing "setup" logic which defensively removes cookies as
a precaution against still other tests which do not properly restore
global state.

This resolves a known instability caused by
cookies/prefix/__secure.document-cookie.https.html. That test sets a
cookie with a "Domain" attribute which
cookies/prefix/__secure.header.https.html is not written to remove.

Avoid test interactions by removing cookies at the completion of each
test. Use the `add_cleanup` feature of testharness.js to ensure that
this occurs regardless of the passing/failing status of each test.
Persist the existing "setup" logic which defensively removes cookies as
a precaution against still other tests which do not properly restore
global state.

This resolves a known instability caused by
`cookies/prefix/__secure.document-cookie.https.html`. That test sets a
cookie with a "Domain" attribute which
`cookies/prefix/__secure.header.https.html` is not written to remove.
@mikewest
Copy link
Member

mikewest commented Sep 4, 2018

Looks totally reasonable to me, thank you for addressing it!

@mikewest mikewest merged commit 88e4a9a into web-platform-tests:master Sep 4, 2018
@jugglinmike
Copy link
Contributor Author

Happy to help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants