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

Start tests of preferences.js to cover basic module structure. #69

Closed
Tracked by #63
acrosman opened this issue Jul 24, 2021 · 6 comments
Closed
Tracked by #63

Start tests of preferences.js to cover basic module structure. #69

acrosman opened this issue Jul 24, 2021 · 6 comments
Labels
enhancement New feature or request good first issue Good for newcomers testing Related to automated testing

Comments

@acrosman
Copy link
Owner

acrosman commented Jul 24, 2021

Currently the src/preferences.js has no test coverage. It would be helpful to have a test class that at least loads the module and validates that the module structure matches the expected. The first two tests covering sf_calls.js provide a pattern to work from.

Any additional test coverage is also welcome and appreciated.

@acrosman acrosman mentioned this issue Jul 24, 2021
7 tasks
@acrosman acrosman added enhancement New feature or request good first issue Good for newcomers testing Related to automated testing labels Jul 24, 2021
@acrosman acrosman changed the title Start tests of preferences.js to cover basic module structure (see first two tests in sf_calls tests), and other simple features. Start tests of preferences.js to cover basic module structure. Jul 26, 2021
@imrishabh18
Copy link
Contributor

I have started with this issue first and made the first test for the validation of exports. Now moving on the next one, test for getCurrentPreferences(). I am getting a error for this line Here , which is EACCES: permission denied, mkdir '/path' . Can you help me with this?

@acrosman
Copy link
Owner Author

acrosman commented Jul 26, 2021

Thanks for work on this! It's great to have more people contributing.

The existing Electron mock provides a fake response for the app.getPath call in line 6 (I had rather forgotten it was there, or would get triggered here, but clearly it does). That's giving you an invalid path when fs-extra tries to make sure the preference file exists.

You can probably create a Jest mock for fs-extra that provides a response to this call and the readfilesync call in line 48 (which assumes the file exists because of the ensurefilesync call that's tripping you up). It's possible there are Jest mock's around in their own library for fs-extra as well, I've never looked. But since you're learning Jest, and there are only a couple functions that need to be mocked, it probably makes sense to give it a try. We don't really want valid file system calls in the testing process anyway -- fs-extra has its own tests for that and would just mean this project's tests needed to do more clean up.

@acrosman
Copy link
Owner Author

acrosman commented Aug 3, 2021

@imrishabh18 if you create a Pull Request with what you have so far I may be able to help you continue to move the contribution forward.

@imrishabh18
Copy link
Contributor

Okay, this is what I have currently.

@acrosman
Copy link
Owner Author

acrosman commented Oct 3, 2021

I've merged PR #77, but there is still plenty of test coverage needed here.

As I noted in comments on that PR:

The next low hanging fruit would be to validate that the internals of the module are correct like in the SF Calls tests. You'll notice that it's using a special __get__() function provided by rewire.

With those done try tests of the action functions one at a time. Start with setWindow() since it doesn't do any type checking nothing about Electron or the existing mocks will matter. Then you will have your feet under you a bit more and can dive into trying to resolve the mock issue we discussed in the main issue.

@acrosman
Copy link
Owner Author

acrosman commented Oct 1, 2022

Closed with #175

@acrosman acrosman closed this as completed Oct 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers testing Related to automated testing
Projects
None yet
Development

No branches or pull requests

2 participants