store icon indicating copy to clipboard operation
store copied to clipboard

test(unit): update unit test setup, revise existing unit tests

Open rfprod opened this issue 2 years ago • 15 comments

  • [x] update jest-preset-angular and respective imports;
  • [x] rename setup-jest files - use conventional kebab-case for file names;
  • [x] revise existing unit tests - address linting issues, add missing types;

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x] The commit message follows our guidelines: https://github.com/ngxs/store/blob/master/CONTRIBUTING.md#commit
  • [ ] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[x] Maintenance: jest-preset-angular was updated to the next minor version, unit tests were updated accordingly. tslib was updated. Angular integration test dependency versions were fixed.

What is the current behavior?

Unit test setup was updated. tslib was updated. Angular integration test dependency versions were fixed. No functional changes have been made.

Issue Number: N/A

What is the new behavior?

Unit test setup is now a bit more up to date. Some linting issues in the spec files were addressed.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

rfprod avatar May 23 '22 17:05 rfprod

@markwhitfeld I'm wondering why CI checks are not running on non-member PRs?

arturovt avatar May 23 '22 18:05 arturovt

@arturovt does this change look good?

rfprod avatar May 24 '22 22:05 rfprod

I will have a look at the CI settings. Don't have my dev machine at the moment, but will check the guthub/circleci settings.

markwhitfeld avatar Jun 06 '22 13:06 markwhitfeld

I will have a look at the CI settings. Don't have my dev machine at the moment, but will check the guthub/circleci settings.

@markwhitfeld what do you think of migrating CI from the CircleCI to the GH Actions CI? I can help with that if needed. I have extensive experience with the GH Actions CI.

rfprod avatar Jun 07 '22 15:06 rfprod

So frustrated with CircleCI. It seems that something was changed to tighten security at the end of April around PRs originating from forked repositories. But, I have checked and all of the settings are correct. PRs to our repo should be running the build steps! So frustrating.

@rfprod Oh, yes please!!!!!! That would be a massive help. We have been intending to do it for a while. It would probably be best to start by just mimicking the build tasks we have in CircleCI. Then we can look at how we can correctly leverage the full power of github actions from there. Could you create a PR for this?

markwhitfeld avatar Jun 07 '22 20:06 markwhitfeld

@rfprod Could you rebase this on master?

markwhitfeld avatar Jun 17 '22 14:06 markwhitfeld

@rfprod Could you rebase this on master?

@markwhitfeld rebased

rfprod avatar Jun 17 '22 15:06 rfprod

BundleMon

Unchanged files (3)
Status Path Size Limits
:white_check_mark: fesm2015/ngxs-store.js
88.88KB 125KB / +0.5%
:white_check_mark: fesm2015/ngxs-store-operators.js
6.23KB 15KB / +0.5%
:white_check_mark: fesm2015/ngxs-store-internals.js
4.22KB 20KB / +0.5%

No change in files bundle size

Unchanged groups (3)
Status Path Size Limits
:white_check_mark: @ngxs/store(esm2015)[gzip]
./esm2015/**/*.js
166.82KB +1%
:white_check_mark: @ngxs/store(umd)[gzip]
./bundles/*.umd.js
35.32KB +1%
:white_check_mark: @ngxs/store(fesm2015)[gzip]
./fesm2015/*.js
24.02KB +1%

Final result: :white_check_mark:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

bundlemon[bot] avatar Jun 22 '22 14:06 bundlemon[bot]

BundleMon

Groups updated (2) Status Path Size Limits @ngxs/store(umd)[gzip] ./bundles/.umd.js 43.87KB (+1.4KB +3.3%) +1% @ngxs/store(umd.min)[gzip] ./bundles/.umd.min.js 12.71KB (-1.27KB -9.1%) +1% Unchanged groups (4) Final result: x

View report in BundleMon website arrow_right

Current branch size history | Target branch size history

@markwhitfeld please take a look. I am not sure how you would prefer to resolve this situation: umd.js became larger, exceeding the threshold, umd.min.js became smaller.

rfprod avatar Jun 22 '22 18:06 rfprod

BundleMon (NGXS Plugins)

Unchanged files (14)
Status Path Size Limits
:white_check_mark: Plugins(umd)[gzip]
storage-plugin/bundles/ngxs-storage-plugin.um
d.js
7.96KB +1%
:white_check_mark: Plugins(umd)[gzip]
router-plugin/bundles/ngxs-router-plugin.umd.
js
7.41KB +1%
:white_check_mark: Plugins(umd)[gzip]
websocket-plugin/bundles/ngxs-websocket-plugi
n.umd.js
6.92KB +1%
:white_check_mark: Plugins(umd)[gzip]
hmr-plugin/bundles/ngxs-hmr-plugin.umd.js
6.89KB +1%
:white_check_mark: Plugins(fesm2015)[gzip]
storage-plugin/fesm2015/ngxs-storage-plugin.j
s
3.64KB +1%
:white_check_mark: Plugins(umd)[gzip]
form-plugin/bundles/ngxs-form-plugin.umd.js
3.41KB +1%
:white_check_mark: Plugins(fesm2015)[gzip]
router-plugin/fesm2015/ngxs-router-plugin.js
3.09KB +1%
:white_check_mark: Plugins(umd)[gzip]
devtools-plugin/bundles/ngxs-devtools-plugin.
umd.js
2.75KB +1%
:white_check_mark: Plugins(fesm2015)[gzip]
form-plugin/fesm2015/ngxs-form-plugin.js
2.65KB +1%
:white_check_mark: Plugins(fesm2015)[gzip]
hmr-plugin/fesm2015/ngxs-hmr-plugin.js
2.65KB +1%
:white_check_mark: Plugins(fesm2015)[gzip]
websocket-plugin/fesm2015/ngxs-websocket-plug
in.js
2.59KB +1%
:white_check_mark: Plugins(umd)[gzip]
logger-plugin/bundles/ngxs-logger-plugin.umd.
js
2.53KB +1%
:white_check_mark: Plugins(fesm2015)[gzip]
devtools-plugin/fesm2015/ngxs-devtools-plugin
.js
2.17KB +1%
:white_check_mark: Plugins(fesm2015)[gzip]
logger-plugin/fesm2015/ngxs-logger-plugin.js
2.01KB +1%

No change in files bundle size

Unchanged groups (3)
Status Path Size Limits
:white_check_mark: All Plugins(esm2015)[gzip]
./-plugin/esm2015/**/.js
108.45KB +1%
:white_check_mark: All Plugins(umd)[gzip]
./-plugin/bundles/.umd.js
37.88KB +1%
:white_check_mark: All Plugins(fesm2015)[gzip]
./-plugin/fesm2015/.js
18.8KB +1%

Final result: :white_check_mark:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

bundlemon[bot] avatar Jun 27 '22 21:06 bundlemon[bot]

BundleMon (Integration Projects)

Unchanged files (3)
Status Path Size Limits
:white_check_mark: Main bundles(Gzip)
hello-world-ng13-ivy/dist-integration/main.(h
ash).js
69.83KB +1%
:white_check_mark: Main bundles(Gzip)
hello-world-ng12-ivy/dist-integration/main.(h
ash).js
67.99KB +1%
:white_check_mark: Main bundles(Gzip)
hello-world-ng14-ivy/dist-integration/main.(h
ash).js
65.5KB +1%

No change in files bundle size

Final result: :white_check_mark:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

bundlemon[bot] avatar Jun 27 '22 21:06 bundlemon[bot]

Ah, it all makes sense now, the reason why we have been having issues with jest in the integration apps is because the integration apps are borrowing the version from the root node modules folder. It does not make sense to do with jest in integration apps because the version of the jest builder will then be mismatched to the angular version being tested. The integration apps should be using the version of jest that was shipped with that version of angular.

I guess it is ok for Cypress to still use the shared version because it is pretty isolated from angular code, and has a large install cost.

markwhitfeld avatar Jul 15 '22 20:07 markwhitfeld

Ah, it all makes sense now, the reason why we have been having issues with jest in the integration apps is because the integration apps are borrowing the version from the root node modules folder. It does not make sense to do with jest in integration apps because the version of the jest builder will then be mismatched to the angular version being tested. The integration apps should be using the version of jest that was shipped with that version of angular.

I guess it is ok for Cypress to still use the shared version because it is pretty isolated from angular code, and has a large install cost.

@markwhitfeld I have several questions, if you will.

How do you want to proceed with dependencies upgrade? Do you plan to eventually upgrade all project dependencies to the latest versions? Do you think it is reasonable to support older versions of Angular that are no longer supported by the core Angular team? https://angular.io/guide/releases#support-policy-and-schedule

Support policy and schedule

Dates are offered as general guidance and are subject to change.

All major releases are typically supported for 18 months.

SUPPORT STAGE SUPPORT TIMING DETAILS
Active 6 months Regularly-scheduled updates and patches are released
Long-term (LTS) 12 months Only critical fixes and security patches are released

Angular versions v2 to v11 are no longer under support.

Support policy and schedule Dates are offered as general guidance and are subject to change.

All major releases are typically supported for 18 months.

SUPPORT STAGE SUPPORT TIMING DETAILS
Active 6 months Regularly-scheduled updates and patches are released
Long-term (LTS) 12 months Only critical fixes and security patches are released

The following table provides the status for Angular versions under support.

VERSION STATUS RELEASED ACTIVE ENDS LTS ENDS
^14.0.0 Active 2022-06-02 2022-12-02 2023-12-02
^13.0.0 LTS 2021-11-04 2022-06-02 2023-05-04
^12.0.0 LTS 2021-05-12 2021-11-12 2022-11-12

Angular versions v2 to v11 are no longer under support.

rfprod avatar Jul 26 '22 22:07 rfprod

@rfprod Thank you so much for walking the long road with this PR. I really appreciate your efforts. And patience!

I know that there is an official support policy for Angular. While we are not bound to support versions outside of that official policy, it is still good to know what versions we do still support. This is one of the purposes of the integration apps. It is not a disaster if an old unsupported version stops working, but at least we know where we stand and can update our official stance on what versions we support.

markwhitfeld avatar Jul 29 '22 11:07 markwhitfeld

The ideal would be that we ensure that the jest and tslib deps match the respective version that the respective version of the Angular CLI would have produced. We can find this by running this on our local machine by running something like npx @angular/cli@11 new ng11 in some folder and then having a look at the package.json produced in the ng11 folder. For each integrations app we can then use the versions specified for jest as well as the angular dependencies. I suspect that this would actually resolve the original jest failures that prompted this PR.

PS. In the long run I would actually like to automate this so that we can run a command in the NGXS project like yarn test:integration 11, where the 11 is a parameter to the script. This would use the npx command above to generate the project to a target folder using the cli and then copy over the integration app files (from a shared location) and patch the package.json file as necessary. It would essentially be an automation of the process above, but using the newly generated folder as the dynamically generated integration app's folder and just the shared integration app files would be in the repo. Once that is in place we could use the github actions matrix to quickly add whatever versions we desire to the build process.

markwhitfeld avatar Jul 29 '22 11:07 markwhitfeld

@rfprod Resurrecting this PR after the upgrade to the new ivy package format. This resulted in some of the fixes for older versions of integration apps not being needed anymore. After merging master I found some small tweaks required and made these changes (I pushed to your branch). If all the checks pass, then I am happy for this to be merged.

markwhitfeld avatar Dec 14 '22 19:12 markwhitfeld

Code Climate has analyzed commit 63f68142 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 97.0% (0.0% change).

View more on Code Climate.

codeclimate[bot] avatar Dec 14 '22 19:12 codeclimate[bot]