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 automerge-repo-react-hooks tests #112

Merged
merged 10 commits into from
Aug 3, 2023
Merged

Conversation

neftaly
Copy link
Collaborator

@neftaly neftaly commented Jul 9, 2023

This PR adds tests for useRepo, using jsdom to mock a browser, via vitest.

@neftaly
Copy link
Collaborator Author

neftaly commented Jul 9, 2023

I was hoping to use a headless browser, so that we can test polyfills for the web navigation API, unfortunately this part of vitest is still experimental, and will not work until browser test isolation lands.

vitest.config.ts:

test: {
    browser: {
      enabled: true,
      headless: true,
      provider: 'playwright',
      name: 'firefox', // Navigation API only supported in Chrome; check FF polyfill works
    }
}

})

test.skip("should return repo from context", () => {
const wrapper = ({ children }) => (
<RepoContext value={repo} children={children} />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This does not work yet, once it does the PR will be ready for review

@@ -34,7 +34,7 @@
"dependencies": {
"cbor-x": "^1.3.0",
"debug": "^4.3.4",
"eventemitter3": "^4.0.7",
"eventemitter3": "^5.0.1",
Copy link
Collaborator Author

@neftaly neftaly Jul 9, 2023

Choose a reason for hiding this comment

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

@neftaly
Copy link
Collaborator Author

neftaly commented Jul 13, 2023

Turns out I needed <RepoContext.Provider> not <RepoContext> -.-

@neftaly neftaly marked this pull request as ready for review July 13, 2023 06:36
@pvh
Copy link
Member

pvh commented Aug 3, 2023

Hey @neftaly, apologies for the delay in getting into this. I think for now we can disable this test in the test runner on the github action server but we'll need to figure out the best way to do that before I can land because it will either crash the tests or leave them hanging if we don't.

@neftaly
Copy link
Collaborator Author

neftaly commented Aug 3, 2023

Are you ok with renaming "yarn test" command to "yarn test-hooks"? Or do you want me to add configuration to actions?

@pvh
Copy link
Member

pvh commented Aug 3, 2023

ah, that's extremely simple and totally fine by me for a stop-gap

@neftaly neftaly mentioned this pull request Aug 3, 2023
12 tasks
@pvh
Copy link
Member

pvh commented Aug 3, 2023

@neftaly I'm going to just do that for you since I don't see a patch yet and it's trivial, then merge. Thank you for this!

@pvh pvh merged commit 293f725 into automerge:main Aug 3, 2023
1 check passed
@pvh
Copy link
Member

pvh commented Aug 3, 2023

Done!

@@ -10,11 +10,22 @@
"main": "dist/index.js",
"scripts": {
"build": "tsc",
"test-browser": "vitest",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this should be "test": "vitest run",

@neftaly
Copy link
Collaborator Author

neftaly commented Aug 3, 2023

Thanks!
I didn't know the CI would pick up this test, sorry. The tests were running in watcher mode. Changing test command to vitest run will fix this.

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.

None yet

2 participants