Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

e2e-test-utils-playwright - RequestUtils - add support for extraHTTPHeaders #62466

Open
Chrico opened this issue Jun 11, 2024 · 5 comments
Open
Labels
[Package] E2E Tests /packages/e2e-tests [Type] Enhancement A suggestion for improvement.

Comments

@Chrico
Copy link
Contributor

Chrico commented Jun 11, 2024

What problem does this address?

Currently the RequestUtils::setup() creates a new Context and only uses baseURL and storageState (by resolving the storageStatePath), but it does not support further options like extraHTTPHeaders or httpCredentials.

What is your proposed solution?

It would be good to enhance the RequestUtils::setup()-method by splitting up the paramaters into "browser context options"-object to support all possible options and the rest (like user) is separated.

So instead we could change it to following:

Edit: Changed from BrowserContextOptions to APIRequestContextOptions as mentioned here: #62466 (comment)

interface APIRequestContextOptions extends Readonly< {
	baseURL?: string,
	extraHTTPHeaders?:  { [ key: string ]: string },
	httpCredentials?: {
		username: string,
		password: string,
		origin?: string
	},
	ignoreHTTPSErrors?: boolean,

	// snip
}> {}


class RequestUtils {

	// snip

	static async setup( context?: APIRequestContextOptions, user?: User, storageStatePath?: string ) {
	   // snip
	}
}

See full list of APIRequest Context Options: https://playwright.dev/docs/api/class-apirequest#api-request-new-context

@Chrico Chrico added the [Type] Enhancement A suggestion for improvement. label Jun 11, 2024
@jordesign jordesign added the [Package] E2E Tests /packages/e2e-tests label Jun 14, 2024
@kopepasah
Copy link
Member

Currently, we have a test site that uses httpCredentials for access, and we are unable to pass any additional options to the setup() method. I agree with @Chrico that these options must be added, and the best case is to pass all available options to Playwright, rather than limit the options available with the WordPress E2E library.

Generally speaking, when a abstraction library is created, it is a good practice to allow passing of any additional options to the underlying library (unless there is a specific reason not to do so).

Does anyone know if there was a specific reason for not allowing all options to be passed (asking just in case there is a caveat to this approach)?

@kopepasah
Copy link
Member

See full list of Browser Context Options: https://playwright.dev/docs/api/class-browser#browser-new-context

@Chrico I think we need to pass the https://playwright.dev/docs/api/class-apirequest#api-request-new-context, correct? Very similar to browser context options, but specific to the APIRequest class.

@Chrico
Copy link
Contributor Author

Chrico commented Jul 3, 2024

@Chrico I think we need to pass the https://playwright.dev/docs/api/class-apirequest#api-request-new-context, correct? Very similar to browser context options, but specific to the APIRequest class.

Yes, you're right, the RequestUtils is not using the Browser and instead the APIRequest. 👍🏻

@Chrico
Copy link
Contributor Author

Chrico commented Jul 11, 2024

@jordesign any feedback regarding that issue? :-)

@jordesign
Copy link
Contributor

@Chrico - thank you, but no feedback - this kind of thing is above my technical expertise :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] E2E Tests /packages/e2e-tests [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

3 participants