xk6-browser
xk6-browser copied to clipboard
Refactor test to use virtual filesystem
This issue was identified in grafana/xk6-browser#323.
The Storage
type was designed to accept different filesystems, specifically for virtual filesystems to be able to test features that work with the FS.
Currently the test to check that the temporary directory is deleted writes and reads of the local FS. This is problematic since the test looks for directories which follow a certain pattern, and if the test were to fail then future tests would fail unless we manually delete the directories that were created. The use of a virtual FS would negate this so that subsequent test runs would not fail due to past failures.
It's also worth making Storage
thread safe by using sync.Once
.
I don't know the context behind why we're doing the way we're doing it.
I'm not sure about it myself. In general, it's a good idea to abstract away FS function calls so that you can mock them out in tests and avoid touching the FS (or use a virtual FS like afero), but from what I've seen we're not using them in our tests, or anywhere else.
@inancgumus originally wrote this, so WDYT? Should we remove
fsMkdirTemp
andfsRemoveAll
and just useos
directly?
Originally posted by @imiric in https://github.com/grafana/xk6-browser/pull/323#discussion_r897719181
Other comments:
- https://github.com/grafana/xk6-browser/pull/323#discussion_r897722737 -- this affirms that it was designed to work with tests
- https://github.com/grafana/xk6-browser/pull/323#discussion_r897723801 -- use
sync.Once
It occurred to me that this diff in #402 can also solve the concurrency problem without needing you to do anything more. 🤔🤞
It occurred to me that this diff in #402 can also solve the concurrency problem without needing you to do anything more. 🤔🤞
Oh nice, I'm working on #402 first, so let's see.