playwright-java icon indicating copy to clipboard operation
playwright-java copied to clipboard

[Feature]: JUnit5: Page and BrowserContext fixture support in BeforeAll/AfterAll methods

Open Sveshais opened this issue 8 months ago • 10 comments

🚀 Feature Request

Current Page and BrowserContext JUnit fixture implementation prevents using these fixtures in BeforeAll/AfterAll methods while APIRequestContext does not have this restriction. This makes using Page for class-wide setup/teardown actions unnecessarily cumbersome.

Please consider supporting either an isolated Page and BrowserContext for static BeforeAll/AfterAll or even shared ones when used with @TestInstance(TestInstance.Lifecycle.PER_CLASS), if possible.

Example

package tests;

import com.microsoft.playwright.*;
import com.microsoft.playwright.junit.Options;
import com.microsoft.playwright.junit.OptionsFactory;
import com.microsoft.playwright.junit.UsePlaywright;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import java.util.regex.Pattern;

import static com.microsoft.playwright.assertions.PlaywrightAssertions.assertThat;

// re-purposing example from the documentation
@UsePlaywright(PlaywrightTest.CustomOptions.class)
public class PlaywrightTest {
    public static class CustomOptions implements OptionsFactory {
        @Override
        public Options getOptions() {
            return new Options()
                    .setHeadless(true)
                    .setContextOptions(new Browser.NewContextOptions()
                            .setBaseURL("https://github.com"))
                    .setApiRequestOptions(new APIRequest.NewContextOptions()
                            .setBaseURL("https://playwright.dev"));
        }
    }

    @BeforeAll
    public static void setup(APIRequestContext request) {
        // this works
        APIResponse response = request.get("/");
        assertThat(response).isOK();
    }

    @Test
    public void testWithCustomOptions() {
        assert true;
    }

//    @AfterAll
//    public static void teardown(Page page) {
//        // ParameterResolutionException
//    }

    @AfterAll
    public static void teardown(Browser browser) {
        // this kinda works, but base url from CustomOptions is not applied
        Page page = browser.newPage();
        // navigating to "/", waiting until "load"
        page.navigate("/");
        assertThat(page).hasURL(Pattern.compile("github"));
    }
}

Motivation

There are cases when using browser would be convenient in test setup/teardown context. Right now this cannot be done by using Page or BrowserContext fixtures directly, but using creating new Page from Browser fixture requires fiddling with options.

Sveshais avatar Apr 24 '25 21:04 Sveshais

cc @uchagani

yury-s avatar Apr 25 '25 18:04 yury-s

The idea is that each test uses its own fresh BrowserContext (and by extension Page), so that it's completely isolated from side effects of the previous tests. This is the same approach we take in Playwright Test in Node.

This makes using Page for class-wide setup/teardown actions unnecessarily cumbersome.

Yes, this is the cost of having well-defined scope for each BrowserContext/Page fixture object. IIUC, what you are asking for is a global per class context / page that would be created in beforeAll and destroyed in afterAll and would leak state between the tests. We have "Serial mode" in Playwright Test which is similar and it is a regret as it makes a lot of stuff more convoluted than necessary. In Playwright Test we recommend using test.step() within the same test instead. I don't think anything like that exists in JUnit though.

As a workaround you could define your own fixture that would apply to the before/afterAll methods. Would that work?

yury-s avatar Apr 25 '25 19:04 yury-s

IIUC, what you are asking for is a global per class context / page that would be created in beforeAll and destroyed in afterAll and would leak state between the tests.

Not necessarily. What I would like is a possibility to use Page/BrowserContext fixtures in BeforeAll/AfterAll methods with the same class wide Options. I understand that shared state might be undesirable, so having its own isolated object would be fine too. It also feels inconsistent that APIRequestContext can be used in BeforeAll/AfterAll while BrowserContext or Page can't.

As a workaround you could define your own fixture that would apply to the before/afterAll methods. Would that work?

Probably. I am using Browser fixture as a workaround to initiate new BrowserContext and Page, but right now this requires providing my own Browser.NewContextOptions (or somehow retrieving them from ExtensionContext myself) as some options from OptionsFactory implementation are not applied at Browser level, e.g. BaseUrl.

In general, there are 2 scenarios I am trying to address:

  1. performing some setup/teardown steps via browser using Page. This does not require a shared state.
  2. reusing authentication state by authenticating in BeforeAll and then using the state in tests. I suppose this does not require a shared state either, but custom options are applied at class level making it inconvenient to exchange storage state between BeforeAll and Test methods. Am I missing something?

Overall, having dedicated JUnit5 fixtures is nice and much appreciated!

Sveshais avatar Apr 25 '25 19:04 Sveshais

I am using Browser fixture as a workaround to initiate new BrowserContext and Page, but right now this requires providing my own Browser.NewContextOptions (or somehow retrieving them from ExtensionContext myself) as some options from OptionsFactory implementation are not applied at Browser level, e.g. BaseUrl.

I think we can easily solve this by introducing BrowserContextOptionsExtension and moving existing logic from BrowserContextExtension.getContextOptions into that new fixture. Since it returns a new instance of Browser.NewContextOptions every time, I don't see any issues with isolation and using it in beforeAll etc. Will that work for you? If so, feel free to send a PR.

yury-s avatar Apr 25 '25 21:04 yury-s

introducing BrowserContextOptionsExtension and moving existing logic from BrowserContextExtension.getContextOptions into that new fixture.

I think this is a good compromise. The only concern I have is how traces are handled for these object since trace options are also configured at this level. Trace file-related methods would need to be tested to ensure they don't throw exceptions and don't interfere with existing trace files generation.

uchagani avatar Apr 25 '25 23:04 uchagani

actually i misspoke, if we are just exposing that method so the options can be set by the user, it should work just fine as @yury-s said. I think it's a great compromise.

uchagani avatar Apr 26 '25 00:04 uchagani

@yury-s were you thinking that the usage should be something like this?

public class SomeTests {
    @BeforeAll
    public static beforeAll(Browser browser, Browser.NewContextOptions newContextOptions) {
        // NewContextOptions is already configured through the OptionsFactory
        var context = browser.newContext(newContextOptions);
        var page = context.newPage();
        // do setup stuff
        context.close();
    }

    @AfterAll
    public static afterAll(Browser browser, Browser.NewContextOptions newContextOptions) {
        var context = browser.newContext(newContextOptions);
        var page = context.newPage();
        // do teardown stuff
    }
}

@Sveshais would this work for you?

uchagani avatar Apr 28 '25 14:04 uchagani

@yury-s were you thinking that the usage should be something like this?

Yes, exactly like that.

yury-s avatar Apr 28 '25 18:04 yury-s

@Sveshais would this work for you?

Yes, this looks good to me.

Sveshais avatar May 06 '25 19:05 Sveshais

Implemented in #1788

uchagani avatar May 07 '25 02:05 uchagani