Repository navigation
msw: continue handler migration - #1186
Conversation
|
Deployment previews on netlify for branch
|
dati18
left a comment
There was a problem hiding this comment.
a singleton state is created at import time (including myWikis, lastWikiId, user and getEntityImportCalledTimes). For me it's a bit concerning because the state is glocal and not reset between tests or re-renderings, and a test that creates/deletes wikis can affect later tests.
I think a getter/setter pattern is more suitable than module-level constants. For example:
const getMyWikis = () => { //some code }const setMyWiki = (items) => { //some code }
And recomputelastWikiIdfrom the current array when needed, rather than keeping one stale global value.
Not a blocker for a trivial mock, but I would treat it as a test-flakiness issue. If this file is used across multiple UI tests, it might cause some order-dependent failures.
I agree for unit tests this logic is not suitable. Using a (shared) logic for unit tests I think should come after these initial fixes. As mentioned in the other PR, the scope for these was primarily the migration and touchup of the old broken mock service |
7dd0a19 to
5565314
Compare
rosalieper
left a comment
There was a problem hiding this comment.
We(Tom Dat and I) agreed in the Daily that this could me merged and deploy as it is good enough as it is and @dati18 concerns will be addressed in a follow up ticket.
see comment added to the approve
onUnhandledRequest: 'error'so that not-implemented handlers error instead of bypass msw (docs)/api/wiki/entityImport/api/wiki/entityImport/api/wiki/create/api/wiki/delete/api/wiki/logo/update/\/api\/wiki\/setting\/.*?\/update$//api/wiki/details/api/wiki/api/v1/policies/missing/api/v1/policies/currentremoveWiki:splice()mutates, not copies (docs)test@localanymorehttps://phabricator.wikimedia.org/T436530