[Question] Ambiguous page.unroute when not using string

[Smrtnyk]

System info

• Playwright Version: v1.33
• Operating System: Windows, Ubuntu22
• Browser: All
• Other info:

Source code

• [x] I provided exact source code that allows reproducing the issue locally.

Sorry, but I deviated a bit from the issue template. So, I am interested in page.unroute method. I had a look at the playwright tests https://github.com/microsoft/playwright/blob/52feff39b3fedfb1eb7a6ff1b33e53a50d114fe7/tests/page/page-route.spec.ts#L42 what I see it is only tested with strings as url. What I want to achieve is to unroute registered routes after each test. We are using regex for that.

In our POM we have this

registerRoutes(page: Page): Promise<void> {
        return page.route(/(test\.local\/)|(domain2)/, this.testRouteHandler.bind(this));
    }

    async destroy(): Promise<void> {
        await this.page.unroute(/(test\.local\/)|(domain2)/);
    }

Looking at the implementation in pw https://github.com/microsoft/playwright/blob/52feff39b3fedfb1eb7a6ff1b33e53a50d114fe7/packages/playwright-core/src/client/page.ts#L460 it compares urls by strict equality, and then filters out page._routes. So I guess for pure regexes it wont work. What I did is that I saved the url regex into an instance field, so I can pass in a reference Like this:

readonly routeUrl = /(test\.local\/)|(domain2)/;
registerRoutes(page: Page): Promise<void> {
        return page.route(this.routeUrl, this.testRouteHandler.bind(this));
    }

    async destroy(): Promise<void> {
        await this.page.unroute(this.routeUrl);
    }

After awaiting the destroy method, page object still has a length of one in _routes, and points to the registered handlers. Our goal is to get rid for all registered routes on the page in destroy method. How to achieve it in regex? can unroute also return a new length or something, or a boolean that confirms that routes are indeed unregistered?

Another thing we have is that sometimes there is a request in route handler that takes some time, and eventually throws It happens that at that time another test is running, and route handler from previous test throws in another test, and then urelated test fails.

What I thought is to collect pending promises from route handles into a local array on POM instance, since we use POM as a fixture, and after use(POMfixture) to await that all promises from that array are resolved before fixture can be teared down. Not sure if you guys have a better advice on how to approach it?

[dgozman]

What I thought is to collect pending promises from route handles into a local array on POM instance, since we use POM as a fixture, and after use(POMfixture) to await that all promises from that array are resolved before fixture can be teared down.

This sounds like a nice solution. Alternatively, if you do not expect your routes to throw during the test execution, just try/catch them all.

[dgozman]

After awaiting the destroy method, page object still has a length of one in _routes, and points to the registered handlers.

This is strange. From my local testing, passing the same RegExp instance removes all routes with the same pattern. That said, we should fix this and allow different instances of the same RegExp.

[Smrtnyk]

This sounds like a nice solution. Alternatively, if you do not expect your routes to throw during the test execution, just try/catch them all.

what happens is that we do some post body validation we test our rum product, that sends beacons with json payload when in route handler and we see that it is a beacon request, we test it against a json schema and throw in case it is invalid. Now, it is in a POM instance, and we throw an Error object. Question is, should we just throw like that in POM? We want to fail the test in case it fails the validation. Or is it a better practice to notify playwright in a different way to fail the test? I see on TestInfo it is possible to set the test status and errorMessages, but we don't have that in POM, we could pass it in when we construct the fixture.

But anyway, issue is that that route handler outlives the test, and then it throws in some other test So I guess if I collect those promises and make sure they are resolved before fixture is teared down should be fine then. If it throws, it throws in the same test. And what I want to do is to unregister all of the routes for the page.

Also, are there some best practices somewhere for cleanup when it comes to page and POM? We also register some console and pageerror handlers in our POM onto the page. Should we also unregister those when fixture is teared down?

I see you committed the fix for Regex. Will that land in 1.34? If so, I can wait and try it out again when it is released, and we can close this ticket then. In case it still doesn't work, I can file a repro.

[dgozman]

Or is it a better practice to notify playwright in a different way to fail the test?

You are doing the right thing! Throwing an error is much better than trying to manipulate TestInfo.

Also, are there some best practices somewhere for cleanup when it comes to page and POM? We also register some console and pageerror handlers in our POM onto the page. Should we also unregister those when fixture is teared down?

Usually, each test has its own page, so you don't have to unregister anything, because the page will be closed anyway. If you are reusing the page, you have to carefully unregister.

I see you committed the fix for Regex. Will that land in 1.34?

Yes.

And what I want to do is to unregister all of the routes for the page.

I think we don't have a way to remove all registered routes, but it looks like a fine idea.

[Smrtnyk]

Thanks @dgozman for your inputs. I am fine if we close this issue