You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A pain point while working with companion is that the jest tests are not independent and they share state.
This makes tests nondeterministic, flaky and sometimes makes changing one test cause other unrelated tests also fail. I believe that unit tests should never depend on each other so this is something we should fix.
This is partly being worked around in getServer (mockserver.js) which calls jest.resetModules every time a server is created.
Obstacles and things to fix in order to make tests independent:
getServer return value is being shared between multiple tests. Example here:
// the order in which getServer is called matters because, once an env is passed,
environment variables are being set on process.env in tests (but not cleared after being set, causing obscure bugs in unrelated tests when changing something). I think we should instead parameterise env variables instead of pulling them from process.env and assigning process.env.COMPANION_XYZ in every test.
metrics middleware has global state and crashes if initialised twice
emitter state is stored globally (singleton) and cannot be changed after initialisation
logger state is stored globally (singleton) and cannot be changed after initialisation - a user hit this when they wanted multiple companions running: How to use multiple S3 buckets #3214 (comment)
redis singleton
... possibly more
I guess the cleanest would be to not have any global state at all, and always sent state that is needed into the classes and functions that need them. However it could become tedious with logger.
The text was updated successfully, but these errors were encountered:
A pain point while working with companion is that the jest tests are not independent and they share state.
This makes tests nondeterministic, flaky and sometimes makes changing one test cause other unrelated tests also fail. I believe that unit tests should never depend on each other so this is something we should fix.
This is partly being worked around in
getServer
(mockserver.js
) which callsjest.resetModules
every time a server is created.Obstacles and things to fix in order to make tests independent:
getServer
return value is being shared between multiple tests. Example here:uppy/packages/@uppy/companion/test/__tests__/preauth.js
Line 20 in 7b74148
process.env
in tests (but not cleared after being set, causing obscure bugs in unrelated tests when changing something). I think we should instead parameterise env variables instead of pulling them fromprocess.env
and assigningprocess.env.COMPANION_XYZ
in every test.metrics
middleware has global state and crashes if initialised twiceemitter
state is stored globally (singleton) and cannot be changed after initialisationlogger
state is stored globally (singleton) and cannot be changed after initialisation - a user hit this when they wanted multiple companions running: How to use multiple S3 buckets #3214 (comment)redis
singletonI guess the cleanest would be to not have any global state at all, and always sent state that is needed into the classes and functions that need them. However it could become tedious with
logger
.The text was updated successfully, but these errors were encountered: