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

Improve user-preferences test #868

Closed
araujoarthur0 opened this issue Oct 15, 2022 · 14 comments · Fixed by #880
Closed

Improve user-preferences test #868

araujoarthur0 opened this issue Oct 15, 2022 · 14 comments · Fixed by #880
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@araujoarthur0
Copy link
Collaborator

araujoarthur0 commented Oct 15, 2022

Describe the current limitation
The jest tests for user-preferences.js at __tests__\__main__\user-preferences.js today is not comprehensive with all the possible preferences. We should add new tests f`or each key available in the window. Check the file to see how we already test a few of the keys: default value -> change value and save -> check that save worked or failed as expected.

@araujoarthur0 araujoarthur0 added enhancement New feature or request good first issue Good for newcomers labels Oct 15, 2022
@ochan12
Copy link
Contributor

ochan12 commented Oct 21, 2022

Hey! I wanted to do this but I'm running into issues when running npm run test:jest looks like the babel traspiling is not being correctly done. I managed to run it using babel-jest instead of @babel/plugin-transform-modules-commonjs. Am I missing some configuration?

On RENDERER tests:

  ● Test suite failed to run

      ● Test suite failed to run

          Jest encountered an unexpected token
          This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.
          By default, if Jest sees a Babel config, it will use that to transform your files, ignoring "node_modules".
          Here's what you can do:
           • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
           • If you need a custom transformation specify a "transform" option in your config.
           • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.
          You'll find more details and examples of these config options in the docs:
          https://jestjs.io/docs/en/configuration.html
          Details:
          .../projects/time-to-leave/__tests__/__renderer__/workday-waiver.js:55
          import { showDialog } from '../../js/window-aux.js';
          ^^^^^^
          SyntaxError: Cannot use import statement outside a module
      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1258:14)

On MAIN tests

  ● Test suite failed to run

    1:undefined
    undefined

      at Object.reject (node_modules/@jest-runner/rpc/build/RPCProcess.js:139:18)
      at Server.emit (node_modules/event-pubsub/es5.js:74:21)
      at Server.gotData (node_modules/node-ipc/dao/socketServer.js:194:14)

@araujoarthur0
Copy link
Collaborator Author

Hi @ochan12, did you install all dependencies using the npm ci; npm install commands from https://github.com/thamara/time-to-leave/blob/main/DEVELOPMENT.md?

@ochan12
Copy link
Contributor

ochan12 commented Oct 21, 2022

Yes I did!
npm ci

....
pre-commit:
pre-commit: Detected an existing git pre-commit hook
pre-commit: Old pre-commit hook backuped to pre-commit.old
pre-commit:

> electron@11.5.0 postinstall /projects/time-to-leave/node_modules/electron
> node install.js


> time-to-leave@2.0.2-dev postinstall /projects/time-to-leave
> patch-package

patch-package 6.4.7
Applying patches...
@jest-runner/electron@3.0.0 ✔
added 2033 packages in 16.683s

npm install

....
patch-package 6.4.7
Applying patches...
@jest-runner/electron@3.0.0 ✔
npm WARN bootstrap@5.1.1 requires a peer of @popperjs/core@^2.10.1 but none is installed. You must install peer dependencies yourself.

added 33 packages from 12 contributors and audited 2066 packages in 13.079s

178 packages are looking for funding
  run `npm fund` for details

found 148 vulnerabilities (4 low, 43 moderate, 74 high, 27 critical)
  run `npm audit fix` to fix them, or `npm audit` for details

@araujoarthur0
Copy link
Collaborator Author

Could you also check the node node -v and npm npm -v versions?
I recently installed the latest on a new machine with Windows 11 and was able to get it working with just the ci + install. Are you on Windows?

@ochan12
Copy link
Contributor

ochan12 commented Oct 21, 2022

Nope MacOS.

node -v
v14.18.2
npm -v
6.14.15

I also tried with newer versions, but same result

@araujoarthur0
Copy link
Collaborator Author

Something that sometimes helps for me is to remove the node_modules folder and run ci + install from scratch.
Also, if you run just the preferences tests npm run test:jest preferences, does it work?

@ochan12
Copy link
Contributor

ochan12 commented Oct 21, 2022

Which node version are you working with?

Nope, same result after npm run test:jest preferences

@araujoarthur0
Copy link
Collaborator Author

Mine are 8.19.2 npm and 18.10.0 node.

@araujoarthur0
Copy link
Collaborator Author

@thamara has done development on a Mac, maybe she could help.

@ochan12
Copy link
Contributor

ochan12 commented Oct 22, 2022

It works if I
npm install --dev babel-jest @babel/core @babel/preset-env

remove the babel config from package.json and create babel.config.js with:

module.exports = {
    presets: [['@babel/preset-env', { targets: { node: 'current' } }]]
};

But I don't think is something you wanna add

@araujoarthur0
Copy link
Collaborator Author

If we don't find the cause, you could potentially use this custom configuration to develop the tests, but only commit the test changes and use the pull request test runs to confirm everything is fine.

@thamara
Copy link
Owner

thamara commented Oct 22, 2022

Hmm, I haven't done any programming in mac for a while now, but the CI we have has been able to run fine on macos, so it's looking like it may be something from your env.
Can you try instead of running npm ci, try npm install? I saw someone with some issues with npm ci, but npm install would work fine.
But as Arthur said above, you can develop this way, but only commit the parts related to the tests.

@ochan12
Copy link
Contributor

ochan12 commented Oct 22, 2022

@thamara No luck on running just npm install, so I just kept the configuration locally. Thanks!

@araujoarthur0
Copy link
Collaborator Author

Thanks @ochan12 😁

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants