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

set up tests for validating various domains #4960

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Conversation

gilluminate
Copy link
Contributor

@gilluminate gilluminate commented Jun 6, 2024

Description Of Changes

Set up tests and instructions to run against various real world TLDs and validate that our cookie logic is sound. Can only be run locally and manually, but handy to keep around for validating any future changes.

CleanShot 2024-06-06 at 10 46 01@2x

Copy link

vercel bot commented Jun 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Jun 6, 2024 4:50pm

Copy link

cypress bot commented Jun 6, 2024

Passing run #8170 ↗︎

0 4 0 0 Flakiness 0

Details:

Merge 8adb392 into dd05206...
Project: fides Commit: 7393679a8c ℹ️
Status: Passed Duration: 00:33 💡
Started: Jun 6, 2024 5:01 PM Ended: Jun 6, 2024 5:02 PM

Review all test suite changes for PR #4960 ↗︎

Copy link
Contributor

@NevilleS NevilleS left a comment

Choose a reason for hiding this comment

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

This is smart!

One minor suggestion is that, sometimes when I write little "leave-behind" tests like this, I like to write something to sanity-check the test preconditions where possible? Basically something that would fail really fast and obviously if you missed the comment and never configured /etc/hosts, because I suspect the test failures that will happen here if you run this without that setup would be a bit cryptic.

Something like...

it("has a locally configured /etc/hosts file containing the domains.json for testing", () => {
  # Visit first domain and check that it loads! If this fails, see instructions above.
  cy.request(`http://${domains[0]}:3001`).should(...<check it resolves to localhost?>)
});

the idea is that test gives a really clear failure

@gilluminate
Copy link
Contributor Author

@NevilleS while I love that idea, unfortunately there's not a convenient way to stop the rest from running if that first test fails. Since the easiest way to run that test is by simply using cy.visit, at the end of the day it's already happening on the first test that runs (and all the rest). If there were a way to kill the rest of the tests on first failure I would do it, but it's going to be redundant without that feature.

@gilluminate gilluminate merged commit c43d02f into main Jun 6, 2024
13 checks passed
@gilluminate gilluminate deleted the domain-cookie-tests branch June 6, 2024 17:38
@NevilleS
Copy link
Contributor

NevilleS commented Jun 6, 2024

@NevilleS while I love that idea, unfortunately there's not a convenient way to stop the rest from running if that first test fails. Since the easiest way to run that test is by simply using cy.visit, at the end of the day it's already happening on the first test that runs (and all the rest). If there were a way to kill the rest of the tests on first failure I would do it, but it's going to be redundant without that feature.

That's fair. The closest thing you have in Cypress is beforeEach, but that can be unnecessarily duplicative. Ship it

Copy link

cypress bot commented Jun 6, 2024

Passing run #8174 ↗︎

0 4 0 0 Flakiness 0

Details:

set up tests for validating various domains (#4960)
Project: fides Commit: c43d02fd52
Status: Passed Duration: 00:36 💡
Started: Jun 6, 2024 5:51 PM Ended: Jun 6, 2024 5:51 PM

Review all test suite changes for PR #4960 ↗︎

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

Successfully merging this pull request may close these issues.

2 participants