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

Add jest setup for site #395

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Add jest setup for site #395

wants to merge 8 commits into from

Conversation

chernylu
Copy link
Contributor

No description provided.

@chernylu chernylu requested a review from johnnyomair October 14, 2024 08:57
@chernylu chernylu self-assigned this Oct 14, 2024
@chernylu chernylu force-pushed the add-site-jest-setup branch from 6405046 to 78b1d4b Compare October 15, 2024 10:55
@nsams
Copy link
Member

nsams commented Oct 15, 2024

Do you have a real world example for a site test?

Copy link
Collaborator

@johnnyomair johnnyomair left a comment

Choose a reason for hiding this comment

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

The tests aren't run in the test workflow. Could you please fix this? Thanks!

@@ -47,12 +50,18 @@
"@graphql-codegen/near-operation-file-preset": "^2.5.0",
"@graphql-codegen/typescript": "^2.0.0",
"@graphql-codegen/typescript-operations": "^2.0.0",
"@testing-library/jest-dom": "^6.5.0",
"@testing-library/react": "^16.0.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we add an example usage for testing library as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Added tests for HiddenIfInvalidLink.tsx. ae2b484

testEnvironment: "jsdom",
setupFilesAfterEnv: ["<rootDir>/jest.setup.ts"],
// Add more setup options before each test is run
// setupFilesAfterEnv: ['<rootDir>/jest.setup.ts'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

This option is defined two lines above 😁 IMO we can remove all comments here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if we should add a tests folder for this.

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 prefer splitting tests and application code, but I can remove the __test__ folder when you don't like it.

@chernylu
Copy link
Contributor Author

Do you have a real world example for a site test?

What do you mean? I wrote tests for HiddenIfInvalidLink.

@nsams
Copy link
Member

nsams commented Oct 28, 2024

What do you mean?

I'm just curious and wanted to know what are you testing (or plan) in actual projects.

@johnnyomair
Copy link
Collaborator

Do you have a real world example for a site test?

We'll probably have few tests in the site in projects. I can think of some use cases for sites with more business logic, e.g., with an integrated shop. But it doesn't hurt to have the setup IMO. We could discuss whether we should have example tests as well.

@nsams
Copy link
Member

nsams commented Oct 28, 2024

We'll probably have few tests in the site in projects.

I'd like to see them :) (we should move this discussion into our slack)

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.

3 participants