Skip to content

Conversation

@exequiel09
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

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] Other... Please describe:

Updates to testing to ensure compatibility to `vitest`

What is the current behavior?

The current behavior is that tests are running perfectly fine on jest. This PR ensures that it runs smoothly on vitest without any errors.

Closes #5020

What is the new behavior?

No new behavior. Just ensures to maintain parity when it runs using vitest

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Screenshot 2025-12-02 at 5 19 07 AM

@netlify
Copy link

netlify bot commented Dec 1, 2025

Deploy Preview for ngrx-io canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit c5519ef
🔍 Latest deploy log https://app.netlify.com/projects/ngrx-io/deploys/69358d124cbc810008419339

@netlify
Copy link

netlify bot commented Dec 1, 2025

Deploy Preview for ngrx-site-v19 ready!

Name Link
🔨 Latest commit c5519ef
🔍 Latest deploy log https://app.netlify.com/projects/ngrx-site-v19/deploys/69358d1210a2100008027d25
😎 Deploy Preview https://deploy-preview-5035--ngrx-site-v19.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@exequiel09
Copy link
Contributor Author

Failed test due to snapshot https://github.com/ngrx/platform/actions/runs/19837913910/job/56839569056?pr=5035#step:8:986

Screenshot 2025-12-02 at 5 24 46 AM

Will attempt to fix 🙏🏻

@exequiel09
Copy link
Contributor Author

Failed test due to snapshot https://github.com/ngrx/platform/actions/runs/19837913910/job/56839569056?pr=5035#step:8:986

Screenshot 2025-12-02 at 5 24 46 AM Will attempt to fix 🙏🏻

Snapshot fixed but build somehow failed. Build failed also on the first run. will look at it.

@exequiel09
Copy link
Contributor Author

exequiel09 commented Dec 1, 2025

Failed test due to snapshot https://github.com/ngrx/platform/actions/runs/19837913910/job/56839569056?pr=5035#step:8:986
Screenshot 2025-12-02 at 5 24 46 AM
Will attempt to fix 🙏🏻

Snapshot fixed but build somehow failed. Build failed also on the first run. will look at it.

This is weird. Build works locally but when run on GH actions, it complains it is missing @ngrx/store 🤔 (https://github.com/ngrx/platform/actions/runs/19838261176/job/56840671614#step:8:206)

Screenshot 2025-12-02 at 5 42 50 AM

The error is not only exclusive to me and it happens also in the PR #5030 (https://github.com/ngrx/platform/actions/runs/19838297217/job/56840791232?pr=5030#step:8:183)

@gabrielguerrero
Copy link
Contributor

Hey @exequiel09 yeah, I just noticed in my PR and saw your comment, not sure why the error, that command nx run entity:build runs fine locally

@exequiel09
Copy link
Contributor Author

exequiel09 commented Dec 2, 2025

Hey @exequiel09 yeah, I just noticed in my PR and saw your comment, not sure why the error, that command nx run entity:build runs fine locally

Hey @gabrielguerrero It might be an issue in the pipeline. I guess we'll have to wait for someone in the NgRx team to look at it and provide feedback.

@exequiel09 exequiel09 force-pushed the test/migrate-jest-to-vitest-router-store branch from a9d05b5 to 32e7d13 Compare December 2, 2025 14:20
@gabrielguerrero
Copy link
Contributor

Hey @exequiel09 yeah, I just noticed in my PR and saw your comment, not sure why the error, that command nx run entity:build runs fine locally

Hey @gabrielguerrero It might be an issue in the pipeline. I guess we'll have to wait for someone in the NgRx team to look at it and provide feedback.

Yeah we might need to wait, I think for some reason it doesnt install the store dependency, is a peerDependency so by default is not install but I imagine this must have work some other way before, maybe somethings change in the pipeline that broke it

@exequiel09 exequiel09 force-pushed the test/migrate-jest-to-vitest-router-store branch 3 times, most recently from 270b81e to c8dd89a Compare December 4, 2025 22:12
@exequiel09
Copy link
Contributor Author

exequiel09 commented Dec 4, 2025

The issue in the CI comes happens when the Nx uses the cached build. But when the dependent project is built locally or is also changed, then the whole PR builds fine. This is evident in this run which makes the content of the packages that is being depended upon, invalid.

My hunch is that all build-package target should have outputs property for it to work correctly: https://nx.dev/docs/guides/tasks--caching/configure-outputs#output-files 🤔

@exequiel09 exequiel09 force-pushed the test/migrate-jest-to-vitest-router-store branch from c8dd89a to ca07063 Compare December 4, 2025 22:22
@exequiel09 exequiel09 force-pushed the test/migrate-jest-to-vitest-router-store branch from 32ff1cc to 700559b Compare December 6, 2025 11:17
Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

Thanks @exequiel09 !
I've merged main into your branch to fix the build errors during the CI.

@exequiel09 exequiel09 force-pushed the test/migrate-jest-to-vitest-router-store branch from 24486bd to 19378fb Compare December 6, 2025 16:14
@exequiel09
Copy link
Contributor Author

Thanks @exequiel09 ! I've merged main into your branch to fix the build errors during the CI.

Noice. The build is working fine now. Thank you @timdeschryver 🙏🏻

@timdeschryver timdeschryver changed the title test(router-store): migrate testing suite from jest to vitest refactor(router-store): migrate unit tests to Vitest Dec 7, 2025
@markostanimirovic markostanimirovic merged commit ee69ba0 into ngrx:main Dec 7, 2025
14 checks passed
@exequiel09 exequiel09 deleted the test/migrate-jest-to-vitest-router-store branch December 7, 2025 15:42
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.

@ngrx/router-store: Migrate Router Store Unit Tests to Vitest

4 participants