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

Unstable tests caused by NgxWebstorage #156

Open
bsautel opened this issue Sep 13, 2021 · 2 comments
Open

Unstable tests caused by NgxWebstorage #156

bsautel opened this issue Sep 13, 2021 · 2 comments

Comments

@bsautel
Copy link

bsautel commented Sep 13, 2021

Versions (please complete the following information):

  • NgxWebstorage: 8.0.0
  • Angular: 12.2.0

Describe the bug
We are using NgxWebstorage in an Angular project. Our project has some component tests (that use Angular TestBed) and some of them are unstable: they randomly fail, I would say half of the time at least one test fail which makes the build fail. It's very painful since each time I have to relauch a build on the CI until it's successful (sometimes I have to retry up to three times).

There are multiple tests that are unstable (not a single one) and there are two different causes, both of them seem to be related to NgxWebstorage:

  1. Sometimes I get some incorrect values. For instance before instantiating the component with TestBed I write a value in the local storage using the LocalStorageService and the component reads that value and behaves differently according to this value. Most of the time the value read is ok, but sometimes, with the exact same code, the value read is undefined. Another example, an action on the component is supposed to write something in the local storage, at the end of the execution we check with the LocalStorageService that the value is correctly saved and that's not the case.
  2. Sometimes I get an invalid_strategy error:

Error: invalid_strategy
at Function.get (node_modules/ngx-webstorage/fesm2015/ngx-webstorage.js:405:1)
at NewFeaturesAvailableDetectorService.set (node_modules/ngx-webstorage/fesm2015/ngx-webstorage.js:500:1)
at NewFeaturesAvailableDetectorService.markNewFeaturesAsRead (src/app/help/new-features-available-detector.service.ts:21:41)
at NewFeaturesComponent.ngOnInit (src/app/help/new-features-help/new-features.component.ts:38:46)
at callHook (node_modules/@angular/core/fesm2015/core.js:2526:1)
at callHooks (node_modules/@angular/core/fesm2015/core.js:2495:1)
at executeInitAndCheckHooks (node_modules/@angular/core/fesm2015/core.js:2446:1)
at refreshView (node_modules/@angular/core/fesm2015/core.js:9480:1)
at renderComponentOrTemplate (node_modules/@angular/core/fesm2015/core.js:9579:1)
at tickRootContext (node_modules/@angular/core/fesm2015/core.js:10810:1)

To Reproduce
That's difficult to say how to reproduce that since it happens randomly.

I noticed that not all the tests that are using NgxWebstorage are unstable. Only some of them randomly fail and I couldn't identify why only some of them are unstable.

Our components use the @LocalStorage and @SessionStorage decorators to read and write the storages. Don't know whether this could be the cause of the issue.

The test code uses the NgxWebstorage module with the default configuration:

NgxWebstorageModule.forRoot()

All the tests that use a storage clear it at the end of their execution.

Expected behavior
I would expect my code to have the same behavior every time it is executed but unfortunately that's not the case.

Desktop (please complete the following information):
That happen both on my desktop and on the CI server, they have the same configuration:

  • OS: Ubuntu/Linux 20.04
  • Browser: Chrome and Firefox (happens on both of them)
  • Version: latest

Note that I am using the Angular CLI to run tests (karma and jasmine) and the tests are executed in parallel on Chrome and Firefox and they randomly fail on both of them.

Additional context
I assume that the system load could be responsible of the randomness, at least for the issue 1. Some bugs whose cause is a synchronization issue can happen for instance only if the execution is very slow. What makes me believe that is:

  1. The SyncStorage class (base class of LocalStorageService and SessionStorageService) delegates the operations to the the StorageStrategy which is async (for instance this.strategy.set(StorageKeyManager.normalize(key), value).subscribe(noop); for the store method). I don't know rxjs enough to know whether it is guaranteed that an observable created directly from the value as here will execute synchronously when it is observed but this may be the cause of synchronization issues.
  2. Our build builds both the frontend and the backend of the application. All the independent build tasks are executed in parallel so there are some other CPU and I/O intensive processes that run in the same time on the same host. I also add that the Angular tests run in parallel on Chrome and Firefox. The host is quite busy when the build is running.

I read the documentation and the code of the NgxStorage module and I did not find many tips regarding how to test some components using this module. Maybe I should use another storage configuration when running the tests? I tried to use a stub strategy as shown here but I got the same behavior. This means I am facing this issue I am using both the real browser storage and a fake in-memory storage.

Sorry for this large description but I wanted to provide as much information as possible. Thanks for building this great module!

@bsautel
Copy link
Author

bsautel commented Sep 16, 2021

I kept investigating to try to understand what is the cause of this issue.

The rxjs documentation states that Observable (of is a builder of an observable) guarantees that the processing will be synchronous unless we use a specific scheduler (but this does not seem to be the case).

I still don't know what is the cause of the issue. It does not seem to be related to rxjs but I keep thinking that it could come from a bad synchronization issue. I'll let you know if I have some new information.

@bsautel
Copy link
Author

bsautel commented Mar 16, 2022

After having spent a lot of time to investigate this issue (that made us lose a lot of time in our development process), I think I could identify its cause.

It appears that the main problem comes from the fact that the @LocalStorage and @SessionStorage decorators are static and not provided by the module via the dependency injection mechanism. When using for instance the LocalStorageService in the tested code, if we don't add NgxWebstorageModule.forRoot() to the modules dependency, there will be an injection error. This forces us to provide the module if we forget it. But when using the decorators, if we don't depend on the module, there will be no error. I think this is responsible of the invalid_strategy error.

For some reasons (including this one), i stopped using the decorators in favor to the services. This showed that in some tests we forgot to provide the module. Most of the time, it is in the tests of components that do not use directly a storage but embed a sub component or use a service that uses a storage.

Also, during my investigation I tried to use the in-memory storage to be sure to use a fresh new storage at each test by adding providers: [{provide: STORAGE_STRATEGIES, useClass: InMemoryStorageStrategy, multi: true}]. It sounds like it does not do what I want since my test code still writes to the real local and session storage. Maybe I misconfigured it or I did not understand what it is supposed to do?

I ended with adding this global jasmine hook to be sure the local and the session storage are empty when a test starts:

afterEach(() => {
  localStorage.clear();
  sessionStorage.clear();
});

This avoid clearing manually the storage in all the tests that use a storage or depend on a component that use a storage. I have been pushing those changes a few days ago and the tests seem to be stable now. Hope this will really be the case!

To sum up, it appears that the problem came from a misuse of the library. But I think the library could help us to detect that, for instance by failing to start when using the decorators without having configured the module. What do you think about that?

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

No branches or pull requests

1 participant