ngxs / store

🚀 NGXS - State Management for Angular
http://ngxs.io
MIT License
3.54k stars 406 forks source link

🐞[BUG]: store is not correctly reseted with "store.reset" in test #1814

Closed ibelz closed 2 years ago

ibelz commented 2 years ago

Affected Package

testing

Is this a regression?

There was a similar issue in July 2019 (already fixed). But now with Angular 12 it happens again https://github.com/ngxs/store/issues/1180

Please look at the discussions at 9 December 2020 and later. It was already discussed there but without solution: https://github.com/ngxs/store/issues/1180#issuecomment-741726198

Description

The "store.reset" in test does not always work correctly and keeps the old state values

See the sample:


describe('userState', () => {
  let store: Store;

  beforeEach(async () => {
    await TestBed.configureTestingModule({
      declarations: [],
      imports: [NgxsModule.forRoot([UserState])],
    }).compileComponents();

    store = TestBed.inject(Store);
  });

  afterEach(() => {
    store.reset({});
  });

  it('should contain state', () => {
    store.reset({
      userdata: {
        id: 13,
        name: 'John Doe'
      }
    });
    expect(store.snapshot()).toEqual({
      userdata: {
        id: 13,
        name: 'John Doe'
      }
    });
  });

  it('should NOT contain state', () => {
    expect(store.snapshot()).toEqual({});
  });
});

The last test (should NOT contain state) fails, because the store contains still the old values. But it should be reseted because of "store.reset" in afterEach

🔬 Minimal Reproduction

https://github.com/ibelz/ngxs-reset-test-angular12 Please run src/users.state.spec.ts


Libs:
- @angular/core version: 12.1.0
- @ngxs/store version: 3.7.3

Browser:
- [x] Chrome (desktop) Version 97.0.4692.71
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX

For Tooling issues:
- Node version: v16.5.0
- Platform:  Mac

Others:

splincode commented 2 years ago

@ibelz could you help us, send PR and fix it?

ibelz commented 2 years ago

I would really like to do that from the bottom of my heart, but unfortunately I have no clue where I can fix something. Unfortunately I haven't found a workaround either ...

If you have some hints for me where to start, I can take a look...

ibelz commented 2 years ago

I have found a solution for the special constellation in my project.

The issue was based on an issue with the immutability and not on an error in the "store.reset". We have mistakenly used the same mock data again and again without a deep clone of it. The spread operator is not enough here. We now use JSON.parse and JSON.stringify for a deep clone.

Additionally "beforeEach" instead of "afterEach" works.

arturovt commented 2 years ago

Well, I can confirm it works as expected. You may think that the store is persisted between unit tests, but it's not. There's no reason to call store.reset in afterEach since you get a new state within each unit test. You call reset on a previous instance of the Store class, but a new instance is created when the next unit test is run.

The expect(store.snapshot()).toEqual({}); fails because beforeEach configured a new testing module and it initialized the new state with a value of { userdata: { id: 13, ... } }.