Skip to content

Replace _get_url abstract test method with a pytest fixture#751

Draft
sbland wants to merge 1 commit into
mainfrom
750-use-url-fixture-instead-of-abstract-method-in-tests
Draft

Replace _get_url abstract test method with a pytest fixture#751
sbland wants to merge 1 commit into
mainfrom
750-use-url-fixture-instead-of-abstract-method-in-tests

Conversation

@sbland

@sbland sbland commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Description

Replaces the use of the _get_url abstract method with a def url(self)... fixture. This allows accessing other fixture data such as database mocked instances.

Fixes #750

Type of change

  • Documentation (non-breaking change that adds or improves the documentation)
  • New feature (non-breaking change which adds functionality)
  • Optimization (non-breaking, back-end change that speeds up the code)
  • Technical work (non-breaking, change which is work as part of a new feature)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (whatever its nature)

Key checklist

  • All tests pass (eg. python -m pytest)
  • The documentation builds and looks OK (eg. mkdocs serve)
  • Pre-commit hooks run successfully (eg. pre-commit run --all-files)

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added or an issue has been opened to tackle that in the future. (Indicate issue here: # (issue))

@sbland sbland requested a review from AdrianDAlessandro June 22, 2026 20:58
@sbland

sbland commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@AdrianDAlessandro I needed to implement this to do tests where we needed to pass url params to the _get_url method, particularly for beautiful soup tests.

Also there was a failing test on the main branch. I'm not sure if this is just local to me.

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@AdrianDAlessandro AdrianDAlessandro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure exactly what this is trying to fix. The issue says it's to be able to pass arguments to the _get_url method, but that is already done in some places. I've commented on the parts that do it.

Comment on lines -500 to -501
def _get_url(self, **kwargs):
return reverse("skill_detail", kwargs=kwargs)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is what you are trying to do achieved by this existing pattern?

def test_template_used(self, admin_client, url):
"""Test the correct template is used by the GET request."""
with assertTemplateUsed(template_name=self._template_name):
response = admin_client.get(self._get_url(slug=skill.slug))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Results in being able to pass args to reverse like this

@sbland

sbland commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

I'm not sure exactly what this is trying to fix. The issue says it's to be able to pass arguments to the _get_url method, but that is already done in some places. I've commented on the parts that do it.

The existing method works if you are calling _get_url from inside the class that defines get_url but doesn't work when we use _get_url in a fixture. Example here: https://github.com/direct-framework/direct-webapp/blob/main/tests/main/view_utils.py#L63

The use case I couldn't implement is when the url should use params that are defined from mocked database instance objects.

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.

Use url fixture instead of abstract method in tests

2 participants