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

feat: E2E Testing for Planning Constraints #4063

Merged
merged 12 commits into from
Dec 20, 2024
Merged

Conversation

RODO94
Copy link
Contributor

@RODO94 RODO94 commented Dec 10, 2024

What I want to test

  • Planning Constraints can be added for checking all constraints
  • Planning Constraints can be added for checking some constraints
  • It displays correct constraints on public side
  • A user can click to challenge a constraint

I am not going to check UI elements like colours and accordion functionality, as I think this is what Storybook is better at, I'll focus on functionality.

Copy link

github-actions bot commented Dec 10, 2024

Removed vultr server and associated DNS entries

@RODO94
Copy link
Contributor Author

RODO94 commented Dec 10, 2024

@@ -50,6 +50,24 @@ export async function setUpTestContext(
submissionEmail: context.team.settings?.submissionEmail,
},
});
const integrations = await $admin.client.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.

Couldn't find a better way to switch has_planning_data to true - I'll probably move the mutation out and add it to the beforeAll for the geospatial stuff since it's isolated to those tests

.getByRole("button", { name: "Constraints that don't apply" })
.click();

const dontApplyHeadings = await page.getByRole("heading").allTextContents();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

question: should this rather sit in a unit test?

@RODO94 RODO94 marked this pull request as ready for review December 18, 2024 09:27
@RODO94 RODO94 requested a review from a team December 18, 2024 09:27
Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

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

Good start but some unexpected mocks here for me - please see initial comments!

e2e/tests/ui-driven/src/helpers/context.ts Outdated Show resolved Hide resolved
e2e/tests/ui-driven/src/mocks/geospatialMocks.ts Outdated Show resolved Hide resolved
e2e/tests/ui-driven/src/helpers/geoSpatialUserActions.ts Outdated Show resolved Hide resolved
e2e/tests/ui-driven/src/helpers/geoSpatialUserActions.ts Outdated Show resolved Hide resolved
e2e/tests/ui-driven/src/mocks/geospatialMocks.ts Outdated Show resolved Hide resolved
@RODO94 RODO94 force-pushed the rory/e2e-plan-constraints branch from 20a1f19 to e371275 Compare December 18, 2024 13:08
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separated this from the geospatialMocks object as it was quite long and not all of it was super relevant. Felt like the right way to keep things concise

Copy link
Member

Choose a reason for hiding this comment

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

nit: our GIS API which you're mocking here explicitly returns constraints AND metadata in the same response - if you're worried about length I'd much prefer to see the whole planning constraints mock in its' own file rather than split piecemeal like this?

@@ -194,13 +193,6 @@ export async function submitCardDetails(page: Page) {
await page.locator("#confirm").click();
}

export async function answerFindProperty(page: Page) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this to a separate geoSpatialUserActions doc to fit it better with the other ones

Copy link
Member

Choose a reason for hiding this comment

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

nit: our GIS API which you're mocking here explicitly returns constraints AND metadata in the same response - if you're worried about length I'd much prefer to see the whole planning constraints mock in its' own file rather than split piecemeal like this?

@RODO94 RODO94 merged commit 7df5452 into main Dec 20, 2024
12 checks passed
@RODO94 RODO94 deleted the rory/e2e-plan-constraints branch December 20, 2024 13:07
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