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

Migrate from jest to vitest (wip) #92

Draft
wants to merge 32 commits into
base: main
Choose a base branch
from
Draft

Migrate from jest to vitest (wip) #92

wants to merge 32 commits into from

Conversation

ayushmanchhabra
Copy link
Contributor

Fixes: #82

@ayushmanchhabra ayushmanchhabra changed the title chore(test): migrate from jest to vitest (wip) Migrate from jest to vitest (wip) Jun 22, 2024
vitest.config.js Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
tests/setup.js Outdated Show resolved Hide resolved
tests/setup.js Outdated Show resolved Hide resolved
@ayushmanchhabra
Copy link
Contributor Author

I've noticed that the failing tests have a pattern:

Object {
    "customLogger": [Function spy],
    "linux": Object {
      "filePath": "/home/DUMMY/file.ext",
-     "outputPath": "/home/DUMMY/folder/file.desktop",
+     "outputPath": "/home/ayushmanchhabra/Desktop/file.desktop",
    },
    "onlyCurrentOS": true,
    "verbose": true,
  }

How can we mock the dir path?

@ayushmanchhabra
Copy link
Contributor Author

@TheJaredWilcurt ping

tests/setup.js Outdated
if (os.platform() !== 'win32') {
import testHelpers from './testHelpers.js';

if (process.platform !== 'win32') {
Copy link
Member

Choose a reason for hiding this comment

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

I don't remember why, but I do remember setting up all the process.platform/os.platform stuff was done intentionally and was annoying to get working. Not sure why, but I think it was more of a cross-platform node thing, than a jest thing. so would probably still be needed, but not sure.

Copy link
Contributor Author

@ayushmanchhabra ayushmanchhabra Jun 25, 2024

Choose a reason for hiding this comment

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

Setup file useful for running commands which are common to the whole test case - such as afterAll, beforeEach, etc

@@ -3,19 +3,17 @@
* @author TheJaredWilcurt
*/

jest.mock('os');
Copy link
Member

Choose a reason for hiding this comment

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

I think it was related to this, where we mock out the OS module to fake which OS we are on for certain tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some modules are mocked successfully while others aren't such as node:os - will look into it when I get time

Copy link
Member

Choose a reason for hiding this comment

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

I think I mocked out one way of checking the OS/platform, and exclusively used that way of checking for it in the library. That way I could pretend to be a different OS in the tests. But also I still needed a way to check the real OS that was running the tests, because it would have different behavior and the tests still needed to pass on all real OS's. So I used a different way to check the real OS that wasn't mocked. I think that's what was going on with process.platform vs require('os').platform().

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.

Convert Jest tests to Vitest
2 participants