Skip to content

tests: convert util.pathprefix2dict tests from unittest to pytest#6433

Merged
aciba90 merged 4 commits into
canonical:mainfrom
Aamir017:add-tests-for-pathprefix2dict
Sep 1, 2025
Merged

tests: convert util.pathprefix2dict tests from unittest to pytest#6433
aciba90 merged 4 commits into
canonical:mainfrom
Aamir017:add-tests-for-pathprefix2dict

Conversation

@Aamir017
Copy link
Copy Markdown
Contributor

Summary

Added unit tests for util.pathprefix2dict.

Changes

  • Added test_required_only to verify required keys work correctly.
  • Added test_required_missing to ensure errors are raised when required keys are missing.
  • Added test_no_required_and_optional for empty input handling.
  • Added test_required_and_optional to validate both required and optional keys.

Result

All tests pass (pytest -v tests/unittests/test_pathprefix2dict.py).
This improves test coverage for the function.

@Aamir017
Copy link
Copy Markdown
Contributor Author

Hello 👋
This is my first contribution to cloud-init.
I added a small set of unit tests for util.pathprefix2dict.
Looking forward to your feedback — and happy to adjust if needed. Thanks!

@Aamir017
Copy link
Copy Markdown
Contributor Author

Hey @aciba90 can you look at this PR and let me know if there are any changes needed.

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, @Aamir017!

I left an in-line request. In addition to that, you are no adding new test but converting them to pytest, could you please modify the commit message accordingly?

Comment thread tests/unittests/test_pathprefix2dict.py Outdated
self.tmp = tempfile.mkdtemp()
self.addCleanup(shutil.rmtree, self.tmp)
@pytest.fixture
def tmpdir_path(tmp_path):
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 fixuture is unnecessary, could you please directly use tmp_path in each test?

@Aamir017 Aamir017 changed the title Add unit tests for util.pathprefix2dict tests: convert util.pathprefix2dict tests from unittest to pytest Aug 29, 2025
@Aamir017
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @aciba90
I’ve updated the tests to remove the unnecessary fixture and now use tmp_path directly.
Also updated the commit message to clarify that this PR converts the existing tests to pytest rather than adding new ones.

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! I think you didn't push you latest changes.

from tests.unittests.helpers import populate_dir


class TestPathPrefix2Dict(TestCase):
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.

Could we also keep the class grouping?

@Aamir017 Aamir017 force-pushed the add-tests-for-pathprefix2dict branch from 015de02 to 009f7e6 Compare August 29, 2025 14:26
@Aamir017
Copy link
Copy Markdown
Contributor Author

hey @aciba90, can you check this once and lemme know for any other changes

@Aamir017 Aamir017 requested a review from aciba90 August 29, 2025 15:08
@Aamir017
Copy link
Copy Markdown
Contributor Author

hey @aciba90, can you check this once and lemme know for any other changes

@aciba90 aciba90 self-assigned this Sep 1, 2025
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 12ab359 into canonical:main Sep 1, 2025
21 checks passed
DarkPhily pushed a commit to hetznercloud/cloud-init that referenced this pull request Sep 2, 2025
blackboxsw pushed a commit to blackboxsw/cloud-init that referenced this pull request Sep 3, 2025
blackboxsw pushed a commit to blackboxsw/cloud-init that referenced this pull request Sep 3, 2025
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