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

refactor: Tests cleanup and better test isolation #452

Merged
merged 8 commits into from
Apr 25, 2024

Conversation

JuroOravec
Copy link
Collaborator

@JuroOravec JuroOravec commented Apr 23, 2024

  • Removed the use of dedent() in test assertions that used assertHTMLEqual
  • Components are now registered inside tests (e.g. in setUpClass method), and the component registry is cleared between tests.
  • Turned out that autodiscovery is also a stateful action, because the code in the imported python files is executed only once - in other words, if the file/module as already been loaded, then the code will not be run. For this I added a helper to clean up after autodiscovery, so it can be run in multiple tests. See https://github.com/EmilStenstrom/django-components/pull/452/files#diff-9d24dc3dc9330ef153577f547b4024b50a095ce91290796e44b7a59dcd7b9119R65
    • Note: I also updated the autodiscover function to accept an optional mapping function for the import paths. This was needed because in tests, the components dir is not at the top-level, but under tests/.
  • There was also one duplicate test that I removed

Closes #436

@JuroOravec JuroOravec changed the title refactor: clear component registry between tests refactor: Tests cleanup and better test isolation Apr 23, 2024
@@ -150,14 +162,6 @@ def test_replace_slot_in_view(self):
response.content,
)

def test_replace_slot_in_view_with_insecure_content(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we still test this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, forgot to attach a comment here, but this is a duplicate, it has the same function body as the function below (test_replace_slot_in_view_with_insecure_content)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But thanks for pointing out, as I'm looking at it now, I guess the first functions probavbly should have used /test_context_insecure/ endpoint instead?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, you're right. I think I made a typo when I wrote that 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Haha, happens :D Reverted the deletion and fixed the test in 8cc12ef 🎉

@JuroOravec JuroOravec merged commit 091da26 into EmilStenstrom:master Apr 25, 2024
6 checks passed
@JuroOravec JuroOravec deleted the jo-436-update-tests branch April 25, 2024 12:20
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.

Tests: Component registry is not isolated across tests
2 participants