feat: supports custom predicate as the "quiet" option value #424

balavishnuvj · 2020-10-13T18:25:30Z

Closes Predicate for Logs #420

src/setupWorker/start/utils/activateMocking.ts

marcosvega91

Hey @balavishnuvj thanks for working on this great feature ! :)

I have a suggestion about the project structure. Let me know that you think :)

src/setupWorker/start/utils/activateMocking.ts

marcosvega91 · 2020-10-13T19:58:43Z

test/msw-api/setup-worker/start/quiet.fn.true.test.ts

+import * as path from 'path'
+import { TestAPI, runBrowserWith } from '../../../support/runBrowserWith'
+import { captureConsole } from '../../../support/captureConsole'
+
+let runtime: TestAPI
+
+beforeAll(async () => {
+  runtime = await runBrowserWith(
+    path.resolve(__dirname, 'quiet.fn.true.mocks.ts'),
+  )
+})
+
+afterAll(() => {
+  return runtime.cleanup()
+})
+
+test('log the captured request when the "quiet" option is returns to "false"', async () => {
+  const { messages } = captureConsole(runtime.page)
+
+  await runtime.page.evaluate(() => {
+    // @ts-ignore
+    return window.__MSW_REGISTRATION__
+  })
+
+  await runtime.reload()
+
+  const activationMessage = messages.startGroupCollapsed.find((text) => {
+    return text.includes('[MSW] Mocking enabled.')
+  })
+  expect(activationMessage).toBeFalsy()
+
+  const res = await runtime.request({
+    url: `${runtime.origin}/user`,
+  })
+
+  const headers = res.headers()
+  const body = await res.json()
+
+  expect(headers).toHaveProperty('x-powered-by', 'msw')
+  expect(body).toEqual({
+    firstName: 'John',
+    age: 32,
+  })
+
+  const requetsLog = messages.startGroupCollapsed.find((text) => {
+    return text.includes('[MSW]') && text.includes('GET /user')
+  })
+  expect(requetsLog).toBeTruthy()
+})
+
+test('does not log the captured request when the "quiet" option is returns to "true"', async () => {
+  const { messages } = captureConsole(runtime.page)
+
+  await runtime.page.evaluate(() => {
+    // @ts-ignore
+    return window.__MSW_REGISTRATION__
+  })
+
+  await runtime.reload()
+
+  const activationMessage = messages.startGroupCollapsed.find((text) => {
+    return text.includes('[MSW] Mocking enabled.')
+  })
+  expect(activationMessage).toBeFalsy()
+
+  const res = await runtime.request({
+    url: `${runtime.origin}/foo`,
+  })
+
+  const headers = res.headers()
+  const body = await res.json()
+
+  expect(headers).toHaveProperty('x-powered-by', 'msw')
+  expect(body).toEqual({
+    firstName: 'John',
+    age: 32,
+  })
+
+  const requetsLog = messages.startGroupCollapsed.find((text) => {
+    return text.includes('[MSW]') && text.includes('GET /foo')
+  })
+
+  expect(requetsLog).toBeUndefined()
+})


I think that we can integrate this test in quiet.test.ts.

we can update the quiet.mock.ts as

import { setupWorker, rest } from 'msw' const worker = setupWorker( rest.get('/user', (req, res, ctx) => { return res( ctx.json({ firstName: 'John', age: 32, }), ) }), rest.get('/foo', (req, res, ctx) => { return res( ctx.json({ firstName: 'John', age: 32, }), ) }), ) // @ts-ignore window.__MSW_START__ = worker.start

the in the quiet.test.ts we can do as

+ await runtime.reload() + await runtime.page.evaluate(() => { // @ts-ignore - return window.__MSW_REGISTRATION__ + return window.__MSW_START__({ + // Disable logging of matched requests into browser's console + quiet: function (req) { + if (req) { + return req.url.href.includes('foo') + } + return true + }, + }) }) - await runtime.reload()

In this way we can call the function __MSW_START__ with the right options. What do you think?

yeah this looks great, will update. What does await runtime.reload() do? Do I have to remove it?

it will reload the page, because we have a single instance of puppeteer it is necessary to have everything clean between test. It should be called before the evaluate.

So when the test runs it will reload the page and then start MSW.

I think that we can add in a beforeEach instead

beforeEach(() => { await runtime.reload() })

We can also introduce a different test suite like quiet-predicate to scope the predicate functionality.

both works for me, I created new file because I thought that was convention that being followed.
eg. wait-until-ready.false.mocks.ts and wait-until-ready.error.mocks.ts
let me know which works better?

kettanaito · 2020-10-14T09:48:32Z

src/setupWorker/glossary.ts

-  quiet?: boolean
+  quiet?:
+    | boolean
+    | ((req?: MockedRequest, res?: ResponseWithSerializedHeaders) => boolean)


By the time we analyze the quiet option and output a successful mock to console both req and res exist. No need to annotate them as optional with ?.

this was to handle src/setupWorker/start/utils/activateMocking.ts, there quiet can be a function without and req and res. Or am I missing anything?

Yeah, I understand the intention. As I've stated earlier, such internal events like ACTIVATE_MOCKING shouldn't be controlled by the predicate function of the quiet option.

Should we remove the quiet check from this log altogether?

That's a sensible approach, yet it would be a breaking change then. Previously this log was also controlled by options.quiet.

Alternatively, maybe we should distribute a function-as-value in a different option?

yeah, it makes sense to introduce different option to handle this case.
but then usage seems a bit tricky,

{ quiet: true, quietPredicate: (req) => { return req.url.href.includes('profile') }, }

what is expected might be a not sure.

we could ntroducte another option for internal logs

quiet: (req) => { return req.url.href.includes('profile') }, quietInternals: true

we can come up with better names for sure 😅

kettanaito · 2020-10-14T09:50:13Z

src/setupWorker/start/utils/activateMocking.ts

@@ -7,7 +7,9 @@ export const activateMocking = async (
  context.worker?.postMessage('MOCK_ACTIVATE')

  return context.events.once('MOCKING_ENABLED').then(() => {
-    if (!options?.quiet) {
+    const isQuiet =
+      typeof options?.quiet === 'function' ? options?.quiet() : options?.quiet


I don't think we should execute a quiet predicate function here. How about we only check the options.quiet for truthiness?

// Basically, keeping the previous implementation. if (!options?.quiet)

This is an internal log message not bound to any request. It should still abide by quiet: true in case you want to silence all logs, but it shouldn't support predicate as there is no request context to execute the predicate in.

What are your thoughts on this?

wouldn't the options?.quiet be a function?

A good point, as a function value produces a truthy value, while the option describes a negative value (true for disable).

This raises an entirely new concern: how can one have a custom logging predicate for requests and also disable the internal messages? The current quiet option value cannot handle this.

either we make req and res in callback optional or introduce a new option

kettanaito · 2020-10-14T09:51:53Z

test/msw-api/setup-worker/start/quiet.fn.true.mocks.ts

+      }),
+    )
+  }),
+  rest.get('/foo', (req, res, ctx) => {


I'd suggest to use realistic paths so that these usage examples resemble actual usage.

kettanaito · 2020-10-14T09:52:25Z

test/msw-api/setup-worker/start/quiet.fn.true.test.ts

+import * as path from 'path'
+import { TestAPI, runBrowserWith } from '../../../support/runBrowserWith'
+import { captureConsole } from '../../../support/captureConsole'
+
+let runtime: TestAPI
+
+beforeAll(async () => {
+  runtime = await runBrowserWith(
+    path.resolve(__dirname, 'quiet.fn.true.mocks.ts'),
+  )
+})
+
+afterAll(() => {
+  return runtime.cleanup()
+})
+
+test('log the captured request when the "quiet" option is returns to "false"', async () => {
+  const { messages } = captureConsole(runtime.page)
+
+  await runtime.page.evaluate(() => {
+    // @ts-ignore
+    return window.__MSW_REGISTRATION__
+  })
+
+  await runtime.reload()
+
+  const activationMessage = messages.startGroupCollapsed.find((text) => {
+    return text.includes('[MSW] Mocking enabled.')
+  })
+  expect(activationMessage).toBeFalsy()
+
+  const res = await runtime.request({
+    url: `${runtime.origin}/user`,
+  })
+
+  const headers = res.headers()
+  const body = await res.json()
+
+  expect(headers).toHaveProperty('x-powered-by', 'msw')
+  expect(body).toEqual({
+    firstName: 'John',
+    age: 32,
+  })
+
+  const requetsLog = messages.startGroupCollapsed.find((text) => {
+    return text.includes('[MSW]') && text.includes('GET /user')
+  })
+  expect(requetsLog).toBeTruthy()
+})
+
+test('does not log the captured request when the "quiet" option is returns to "true"', async () => {
+  const { messages } = captureConsole(runtime.page)
+
+  await runtime.page.evaluate(() => {
+    // @ts-ignore
+    return window.__MSW_REGISTRATION__
+  })
+
+  await runtime.reload()
+
+  const activationMessage = messages.startGroupCollapsed.find((text) => {
+    return text.includes('[MSW] Mocking enabled.')
+  })
+  expect(activationMessage).toBeFalsy()
+
+  const res = await runtime.request({
+    url: `${runtime.origin}/foo`,
+  })
+
+  const headers = res.headers()
+  const body = await res.json()
+
+  expect(headers).toHaveProperty('x-powered-by', 'msw')
+  expect(body).toEqual({
+    firstName: 'John',
+    age: 32,
+  })
+
+  const requetsLog = messages.startGroupCollapsed.find((text) => {
+    return text.includes('[MSW]') && text.includes('GET /foo')
+  })
+
+  expect(requetsLog).toBeUndefined()
+})


We can also introduce a different test suite like quiet-predicate to scope the predicate functionality.

balavishnuvj · 2020-10-15T10:45:15Z

@kettanaito @marcosvega91 please do let me know your concerns are not addressed.

marcosvega91

LGTM :) thanks :)

balavishnuvj · 2020-12-01T12:04:19Z

@kettanaito what needs to be done here? bit confused 😅

lukesmurray · 2021-02-10T17:46:03Z

looks like a simpler quiet: boolean was merged here. Not sure what the status of this is.

kettanaito · 2021-02-12T23:35:48Z

@balavishnuvj, sorry, I've lost the context of these changes after a while. Worry not, I will check what's left to do, if anything, and update the review status. The issues/active collaborators ratio is rather high, so certain features although useful and awesome don't get as much attention as they deserve.

kettanaito · 2021-02-12T23:37:07Z

@lukesmurray, hey 👋 You're right, you can already silence the logging by passing quiet: false to either worker.start() or server.listen(). This pull request aims for that quiet option to support a custom predicate, so you may choose which requests to log and which to ignore.

kettanaito · 2024-03-17T11:53:59Z

Thanks once again for working on this, @balavishnuvj!

I think I will close this one. I can see this being a useful feature but it has a few cost points that don't quite justify the gain here. It's an additional config, and I'd like to be extremely strict when it comes to it. Moreover, this hasn't been requested as a missing feature for three years the pull request has been open.

I'd be glad to circle back on this if more people find this missing. Until then, closing.

balavishnuvj commented Oct 13, 2020

View reviewed changes

src/setupWorker/start/utils/activateMocking.ts Show resolved Hide resolved

marcosvega91 requested changes Oct 13, 2020

View reviewed changes

kettanaito reviewed Oct 14, 2020

View reviewed changes

kettanaito changed the title ~~feat: add qiet as predicate function~~ feat: supports custom predicate as the "quiet" option value Oct 14, 2020

marcosvega91 previously approved these changes Oct 15, 2020

View reviewed changes

balavishnuvj added 3 commits October 20, 2020 12:02

feat: add qiet as predicate function

26a724c

remove unwanted log

b204f3a

update file name

c12dea9

kettanaito dismissed marcosvega91’s stale review via c12dea9 October 20, 2020 10:02

kettanaito force-pushed the feat/quiet-as-fn branch from 3e8a8c1 to c12dea9 Compare October 20, 2020 10:02

setupWorker: Marks "req" and "res" as required in "quiet" callback

e67d885

kettanaito force-pushed the main branch 3 times, most recently from 548fbd0 to fed3b21 Compare March 11, 2022 15:13

cah-brian-gantzler mentioned this pull request Apr 9, 2022

Decouple from pretender, support MSW others? miragejs/miragejs#1013

Closed

kettanaito force-pushed the main branch from 1a36ada to 5c90799 Compare May 17, 2022 12:25

kettanaito closed this Mar 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: supports custom predicate as the "quiet" option value #424

feat: supports custom predicate as the "quiet" option value #424

balavishnuvj commented Oct 13, 2020 •

edited by kettanaito

Loading

marcosvega91 left a comment

marcosvega91 Oct 13, 2020 •

edited

Loading

balavishnuvj Oct 13, 2020

marcosvega91 Oct 13, 2020

marcosvega91 Oct 13, 2020

kettanaito Oct 14, 2020

balavishnuvj Oct 14, 2020

kettanaito Oct 14, 2020

balavishnuvj Oct 14, 2020

kettanaito Oct 15, 2020

balavishnuvj Oct 17, 2020

kettanaito Oct 20, 2020

balavishnuvj Oct 23, 2020

kettanaito Oct 14, 2020

balavishnuvj Oct 14, 2020

kettanaito Oct 14, 2020

kettanaito Oct 20, 2020

balavishnuvj Oct 20, 2020

kettanaito Oct 14, 2020

kettanaito Oct 14, 2020

balavishnuvj commented Oct 15, 2020

marcosvega91 left a comment

balavishnuvj commented Dec 1, 2020

lukesmurray commented Feb 10, 2021

kettanaito commented Feb 12, 2021

kettanaito commented Feb 12, 2021

kettanaito commented Mar 17, 2024

feat: supports custom predicate as the "quiet" option value #424

feat: supports custom predicate as the "quiet" option value #424

Conversation

balavishnuvj commented Oct 13, 2020 • edited by kettanaito Loading

marcosvega91 left a comment

Choose a reason for hiding this comment

marcosvega91 Oct 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balavishnuvj commented Oct 15, 2020

marcosvega91 left a comment

Choose a reason for hiding this comment

balavishnuvj commented Dec 1, 2020

lukesmurray commented Feb 10, 2021

kettanaito commented Feb 12, 2021

kettanaito commented Feb 12, 2021

kettanaito commented Mar 17, 2024

balavishnuvj commented Oct 13, 2020 •

edited by kettanaito

Loading

marcosvega91 Oct 13, 2020 •

edited

Loading