valendres / playwright-msw

A Mock Service Worker API for Playwright
MIT License
166 stars 23 forks source link

Reduce noise in playwright report #28

Closed solomonhawk closed 2 years ago

solomonhawk commented 2 years ago

Hello, thanks for putting this together. It's nice to have a tidy fixture-based MSW integration.

I'm seeing a fairly noisy output within my playwright reports that's generated from playwright-msw. Is it possible I've got something configured incorrectly or is there a way to prevent playwright-msw from generating so much noise?

I'm seeing several screens of the following in each of my playwright reports, which dwarfs the actual steps taken within the individual spec:

image

My test helper looks like:

import { test as base, expect } from '@playwright/test';
import { createWorkerFixture } from 'playwright-msw';
import type { MockServiceWorker } from 'playwright-msw';

const test = base.extend<{
  worker: MockServiceWorker;
}>({
  worker: createWorkerFixture(),
});

export { test, expect };

The spec in question (login.spec.ts) is pretty barebones:

import { expect, test } from '../helpers/test';
import { rest } from 'msw';

const user = {
  name: 'John Doe'
}

test('user can log in', async ({ page, worker }) => {
  await worker.use(
    rest.post('/auth/login', (_req, res, ctx) => {
      return res(
        ctx.status(200),
        ctx.json({
          success: true,
          data: user,
        }),
      );
    }),
  );

  await page.goto('/');

  const loginBtn = page.getByText('Log in');

  await expect(loginBtn).toHaveAttribute('href', '/login');

  await loginBtn.click();

  await page.getByLabel('Email').fill('john@doe.com');
  await page.getByLabel('Password').fill('secret');

  await page.getByRole('button', { name: 'Log In' }).click();

  await expect(page.getByText('Welcome John Doe!')).toBeVisible();
});

Is there any particular setting within the playwright config that would be relevant here? Is this output caused by the fact that I'm adding a rest handler within the test itself instead of specifying handlers in the worker fixture when it's created?

Cheers

valendres commented 2 years ago

Hey @solomonhawk, I don't believe you have anything misconfigured here or are doing anything. Unfortunately, the method used for intercepting the requests is unfortunately quite noisy. My apologies for this. I too find it quite noisy. I've tried to find a way wot reduce it in the past, but didn't have much luck. I'll have another stab at it and hopefully find a way to reduce it.

Is there any particular setting within the playwright config that would be relevant here?

Unsure at this stage, but I hope so 🙂

Will post updates soon.

solomonhawk commented 2 years ago

Appreciate the reply! Let me know if there's anything I can do to help.

valendres commented 2 years ago

Do you happen to know of a way to suppress logs in Playwright?

In the cypress world, all of the cypress commands allow you to pass an options object, one of which was {log: false}. This would allow granular control over whether logging was performed on a per cypress command basis: https://docs.cypress.io/api/cypress-api/cypress-log#Examples

I can't seem to find a way to do this in Playwright though. I'd be keen to hear if you have any ideas on how we might achieve the desired result.

solomonhawk commented 2 years ago

I poked around and I don't see any current available API for suppressing or turning off logs. There is a way to customize the logger behavior when programmatically launching a browser but that doesn't seem particularly useful.

Another route would be to use a custom HTML reporter, but that doesn't seem like a terribly maintainable approach either.

It looks like some other folks have similar issues and there was a PR merged early this year that at the very least groups/collapses sequential log messages (https://github.com/microsoft/playwright/pull/11160) which we can see in my image above (the numbers in grey after route.continue in a few spots), but that doesn't really help too much.

Do I understand correctly that we're getting a lot of noise because our page route intercepter is called for every request on the page (assets, images, etc.) even though we might only be targeting one or two specific (xhr) routes?

solomonhawk commented 2 years ago

@valendres Do you think it would be possible/viable to change how playwright-msw interacts with playwright and ditch page.route('**/*', ..) in favor of one call to page.route for each specific handler attached to msw (using the exact path specified)? It seems like that would be less tidy overall, and maybe not possible since you'd need to be able to introspect the calls to rest.get/rest.post configured in msw. Dynamically adding page.route(..) calls during a test seems feasible on it's own though.

Just a stray thought this morning. :)

valendres commented 2 years ago

Thanks for looking into this @solomonhawk. It's unfortunate that there isn't an easy first party way to handle this. I was really excited to hear about the log grouping, until you pointed out that it doesn't work very well 😅

Do I understand correctly that we're getting a lot of noise because our page route intercepter is called for every request on the page (assets, images, etc.) even though we might only be targeting one or two specific (xhr) routes?

Yup, that's correct. It was the easiest way to integrate MSW and playwright.

Do you think it would be possible/viable to change how playwright-msw interacts with playwright and ditch page.route('*/', ..) in favor of one call to page.route for each specific handler attached to msw (using the exact path specified)? It seems like that would be less tidy overall, and maybe not possible since you'd need to be able to introspect the calls to rest.get/rest.post configured in msw. Dynamically adding page.route(..) calls during a test seems feasible on it's own though.

This is an awesome idea. I think it should be feasible, I'm going to try it out this morning. If it does work, it'll probably just require a lot of testing to make sure that we're not introducing any regression. I'll let you know how I go.

p.s. sorry for the delay in responding. I've been super busy getting ready to move to the US 🙂

valendres commented 2 years ago

Hey @solomonhawk, turns out this is feasible. Thanks for the good idea 🎉

I've got a POC implementation here: https://github.com/valendres/playwright-msw/pull/33

It has required a significant refactor to be able to implement this - which I'm hoping achieves what you're after. I have published this as 2.0.0-beta.0. Would you be keen on trying it out?

I should note, that it might be a few days before this can get merged and published. While it seems to work well for Rest handlers, some more work will be required to get it to work with GraphQL.

solomonhawk commented 2 years ago

Awesome! I took a quick look at it seems like a promising approach. I didn't mean to send you down a path towards a large refactor/rewrite, so I hope it isn't too much work.

I'll see if I can test this out soon.

valendres commented 2 years ago

Hey @solomonhawk, I've published v2.0.0 which should reduce the amount of noise that's present in the logs. Due to the nature of this large refactor, I've added many unit and integration tests to help ensure that it has not caused any unexpected regression.

Please note that this version also adds support for GraphQL, which resulted in a breaking change to the createWorkerFixture interface. The V2 Migration Guide should serve as a good reference when upgrading 🙂

I'm going to close this issue for now, please let me know how it goes. If there's any issues, feel free to re-open the issue or create a new one.