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

feat: allow disabling storage persistence #1539

Merged
merged 7 commits into from Sep 15, 2022

Conversation

vladfrangu
Copy link
Member

Resolves the request in #1533

@B4nan
Copy link
Member

B4nan commented Sep 15, 2022

Looking good, just the test is broken. Feel free to merge if the CI is green (maybe also try running e2e tests to be sure, but we dont need to wait for the platform ones to finish).

@vladfrangu
Copy link
Member Author

Test was broken because I forgot to update it when I renamed the option 😬

@B4nan
Copy link
Member

B4nan commented Sep 15, 2022

Now it seems to be timeouting


expect(directoryFiles).toHaveLength(0);
// We check that reading the directory for the store throws an error, which means it wasn't created on disk
await expect(() => readdir(expectedPath)).rejects.toThrow();
Copy link
Member

Choose a reason for hiding this comment

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

FYI the argument does not have to be callback (as long as its async), this should work the same:

Suggested change
await expect(() => readdir(expectedPath)).rejects.toThrow();
await expect(readdir(expectedPath)).rejects.toThrow();

At least that is how I use it, if there is a reason to prefer the callback approach, let me know :]

@B4nan B4nan changed the title feat(MemoryStorage): allow disabling the write to disk functionality feat: allow disabling storage persistence Sep 15, 2022
@B4nan B4nan merged commit f65e3c6 into master Sep 15, 2022
@B4nan B4nan deleted the feat/allow-disabling-writing-to-disk-in-memory-storage branch September 15, 2022 11:46
@B4nan
Copy link
Member

B4nan commented Sep 15, 2022

@B4nan
Copy link
Member

B4nan commented Sep 15, 2022

This looks almost like an undeclare dependency or something... Only platform tests fail.

image

edit: or more like that test dont have the browser pool package linked properly?

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.

None yet

2 participants