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 tests to various hooks #16293

Merged
merged 6 commits into from
Sep 7, 2022
Merged

Add tests to various hooks #16293

merged 6 commits into from
Sep 7, 2022

Conversation

krishnaglick
Copy link
Contributor

@krishnaglick krishnaglick commented Sep 2, 2022

What

Due to the size of the Connection Form Refactor PR I've split out some of the testing done there into its own PR.
Additionally, src/test-utils was created to hold our testutils.ts as well as some mock data.
The AnalyticsProvider was added to our central test render wrapper.
The snapshot tests for useInitialValues will let us know if any changes made to the code there have unexpected consequences.

Recommended reading order

Any, filter out the mock data json files.

🚨 User Impact 🚨

None. Tests will fail or the app will fail to build if this breaks anything.

@krishnaglick krishnaglick requested a review from a team as a code owner September 2, 2022 19:04
@github-actions github-actions bot added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Sep 2, 2022
Copy link
Contributor

@teallarson teallarson left a comment

Choose a reason for hiding this comment

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

Can we add testing for useWorkspace()? That is the only hook I see in the ConnectionFormService that isn't addressed in this PR.

💥 so many tests added. I don't see any gaps I can think of beyond the missing hook.

@teallarson
Copy link
Contributor

I said the incorrect service above, I meant WorkspacesService! We use useCurrentWorkspace in the ConnectionFormService

@teallarson teallarson self-requested a review September 7, 2022 14:17
Copy link
Contributor

@teallarson teallarson left a comment

Choose a reason for hiding this comment

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

looks like the tests pass

@krishnaglick krishnaglick merged commit bc0d7cc into master Sep 7, 2022
@krishnaglick krishnaglick deleted the kc/add-tests branch September 7, 2022 17:57
robbinhan pushed a commit to robbinhan/airbyte that referenced this pull request Sep 29, 2022
* Move testutils to test-utils folder

* Add mock data

* Add analytics provider to test renderer

* Add hooks.test.ts, test useUniqueFormId

* Add tests for mapFormPropsToOperation and useInitialValues
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
* Move testutils to test-utils folder

* Add mock data

* Add analytics provider to test renderer

* Add hooks.test.ts, test useUniqueFormId

* Add tests for mapFormPropsToOperation and useInitialValues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants