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

Unit test/realtime account validation #7974

Merged

Conversation

vnsrz
Copy link
Contributor

@vnsrz vnsrz commented Jun 16, 2023

Adds unit tests for the file realtime_account_validation.js, particularly on the password validation.

This was made for educational purposes.

Technical

To test the JQuery in the initRealtimeValidation method, a setup-jest.js file importing the $ identier had to be created and added to the package.json. As far as I know, this doesn't affect the other tests being ran, as they still passed.

Testing

Run docker compose exec web make test, there should be 17 tests running and passing now, as opposed to 16 before.

Screenshot

Screenshot_88

Stakeholders

@cclauss
Copy link
Contributor

cclauss commented Jun 17, 2023

eslint...................................................................Failed
- hook id: eslint
- exit code: 1

/code/scripts/solr_restarter/index.js
  0:0  warning  File ignored because of a matching ignore pattern. Use "--no-ignore" to override

/code/tests/screenshots/index.js
  0:0  warning  File ignored because of a matching ignore pattern. Use "--no-ignore" to override

/code/tests/setup-jest.js
  2:1   error  'global' is not defined  no-undef
  2:12  error  'global' is not defined  no-undef

/code/.storybook/preview.js
  0:0  warning  File ignored because of a matching ignore pattern. Use "--no-ignore" to override

/code/vendor/js/jquery-form/jquery.form.js
  0:0  warning  File ignored because of a matching ignore pattern. Use "--no-ignore" to override

✖ 6 problems (2 errors, 4 warnings)

/code/.storybook/main.js
  0:0  warning  File ignored because of a matching ignore pattern. Use "--no-ignore" to override

/code/conf/svgo.config.js
  0:0  warning  File ignored because of a matching ignore pattern. Use "--no-ignore" to override

✖ 2 problems (0 errors, 2 warnings)

@cclauss
Copy link
Contributor

cclauss commented Jun 17, 2023

> npx concurrently --group npm:lint:js npm:lint:css

[lint:js] 
[lint:js] > openlibrary@1.0.0 lint:js
[lint:js] > eslint --ext js,vue .
[lint:js] 
[lint:js] 
[lint:js] /home/runner/work/openlibrary/openlibrary/tests/setup-jest.js
[lint:js]   2:1   error  'global' is not defined  no-undef
[lint:js]   2:12  error  'global' is not defined  no-undef
[lint:js] 
[lint:js] ✖ 2 problems (2 errors, 0 warnings)
[lint:js] 
[lint:js] npm run lint:js exited with code 1
[lint:css] 
[lint:css] > openlibrary@1.0.0 lint:css
[lint:css] > stylelint ./**/*.less
[lint:css] 
[lint:css] npm run lint:css exited with code 0
Error: Process completed with exit code 1.

@vnsrz
Copy link
Contributor Author

vnsrz commented Jun 17, 2023

> npx concurrently --group npm:lint:js npm:lint:css

[lint:js] 
[lint:js] > openlibrary@1.0.0 lint:js
[lint:js] > eslint --ext js,vue .
[lint:js] 
[lint:js] 
[lint:js] /home/runner/work/openlibrary/openlibrary/tests/setup-jest.js
[lint:js]   2:1   error  'global' is not defined  no-undef
[lint:js]   2:12  error  'global' is not defined  no-undef
[lint:js] 
[lint:js] ✖ 2 problems (2 errors, 0 warnings)
[lint:js] 
[lint:js] npm run lint:js exited with code 1
[lint:css] 
[lint:css] > openlibrary@1.0.0 lint:css
[lint:css] > stylelint ./**/*.less
[lint:css] 
[lint:css] npm run lint:css exited with code 0
Error: Process completed with exit code 1.

Fixed

@cclauss cclauss added the Theme: Testing Involves work related to testing infrastructure, development, and ops. [managed] label Jul 1, 2023
@jimchamp
Copy link
Collaborator

Blocked by changes to the registration page: #7670.

@jimchamp jimchamp added the State: Blocked Work has stopped, waiting for something (Info, Dependent fix, etc. See comments). [managed] label Jul 10, 2023
Copy link
Collaborator

@jimchamp jimchamp left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, @vnsrz. This looks good to me. I'm removing the Jest setup steps that you have added, as similar code was added by #7962.

Some of the tests could be simplified with a second beforeEach call that sets the variables that are same and used by each test. I'll make some small changes to DRY the tests in another PR. Will mention you as a stakeholder so that you'll see what I mean.

package.json Outdated Show resolved Hide resolved
@jimchamp jimchamp merged commit dcd69f8 into internetarchive:master Aug 24, 2023
@jimchamp
Copy link
Collaborator

Thanks for these new tests, @vnsrz! Apologies for taking so long to review them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
State: Blocked Work has stopped, waiting for something (Info, Dependent fix, etc. See comments). [managed] Theme: Testing Involves work related to testing infrastructure, development, and ops. [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants