Skip to content

Conversation

@jf-x-dev
Copy link
Collaborator

@jf-x-dev jf-x-dev commented Dec 2, 2025

This PR changes how page objects are used in tests.

Currently we have a fixture for every individual page object, and in each test these fixtures are passed as arguments. This is how we use the functionality of each of these page objects.

There are some problems with this approach though:

  • There isn't actually any reason for these fixtures to exist
  • Tests that access many pages end up having a very large number of arguments
  • Adding new page objects requires adding new fixtures etc. which ends up requires changes in multiple files

A simpler approach is to just pass the Playwright page fixture to tests, and then create the necessary page objects in the test (e.g. calling LogInPage(page).log_in()). This allows us to:

  • Remove all page fixtures (mavis/test/fixtures/pages.py deleted)
  • Massively reduce arguments required for each test, making them easier to understand and edit
  • Make adding page objects much easier

@jf-x-dev jf-x-dev requested a review from a team as a code owner December 2, 2025 09:52
Copy link
Collaborator

@thomasleese thomasleese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, I've also noticed that having so many of these fixtures is a bit of a pain to work with!

@jf-x-dev jf-x-dev merged commit b9ba2ce into main Dec 2, 2025
3 of 4 checks passed
@jf-x-dev jf-x-dev deleted the remove-page-fixtures branch December 2, 2025 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants