Skip to content

test: refactor test_reporting.py to use only pytest#6449

Merged
aciba90 merged 3 commits into
canonical:mainfrom
chijioke-ibekwe:pytestify-tests
Sep 3, 2025
Merged

test: refactor test_reporting.py to use only pytest#6449
aciba90 merged 3 commits into
canonical:mainfrom
chijioke-ibekwe:pytestify-tests

Conversation

@chijioke-ibekwe
Copy link
Copy Markdown
Contributor

@chijioke-ibekwe chijioke-ibekwe commented Sep 3, 2025

Proposed Commit Message

test: refactor test_reporting.py to use only pytest

Remove helpers.TestCase inheritance, replace unittest assertions with pytest equivalents, and rename helper methods using pytest naming convention. This change is a refactor only. Test behavior and coverage remains exactly the same.

GH-6427

Additional Context

Test Steps

Run the specific unit tests to verify they still pass:
tox -e py3 -- tests/unittests/reporting/test_reporting.py

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@chijioke-ibekwe
Copy link
Copy Markdown
Contributor Author

@aciba90 Please kindly review

Copy link
Copy Markdown
Contributor

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

Thanks @chijioke-ibekwe for the refactoring, it looks good.

I left an inline request and I think the commit related to test_data.py might be for other PR as it is not reference in the PR comment, or is it intended to be included?

getattr(events.status, "BOGUS")


@skipUnlessJsonSchema()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is need for environments where jsonschema is not installed, could you please bring it back?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are these modifications intended for this PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These were from the previous PR. Not sure why it's still when it's already been merged to main. Should I create a new branch for this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One way to fix it is to do git rebase -i main and drop the unwanted commit. Or a new branch and cherry-pick the top commit from here. Whatever works for you, thanks!

def test_base_reporting_handler_is_abstract(self):
regexp = r".*abstract.*publish_event.*"
self.assertRaisesRegex(TypeError, regexp, handlers.ReportingHandler)
with pytest.raises(TypeError, match=r".*abstract.*publish_event.*"):
Copy link
Copy Markdown
Contributor

@aciba90 aciba90 Sep 3, 2025

Choose a reason for hiding this comment

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

This test tests that ReportingHandler is an abstract class, mypy complains about instantiating an abstract class.

Honestly, I think this test doesn't add much value and I suggest removing it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is noted

@chijioke-ibekwe
Copy link
Copy Markdown
Contributor Author

@aciba90 Kindly re-review

Copy link
Copy Markdown
Contributor

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@aciba90 aciba90 merged commit cd03bf9 into canonical:main Sep 3, 2025
21 checks passed
blackboxsw pushed a commit to blackboxsw/cloud-init that referenced this pull request Sep 3, 2025
Remove helpers.TestCase inheritance, replace unittest assertions with pytest equivalents,
and rename helper methods using pytest naming convention. This change is a refactor only.
Test behavior and coverage remains exactly the same.

canonicalGH-6427
blackboxsw pushed a commit to blackboxsw/cloud-init that referenced this pull request Sep 3, 2025
Remove helpers.TestCase inheritance, replace unittest assertions with pytest equivalents,
and rename helper methods using pytest naming convention. This change is a refactor only.
Test behavior and coverage remains exactly the same.

canonicalGH-6427
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