Skip to content

test: unit tests for get_package and get_partitions methods - #56

Draft
cmatsuoka with Copilot wants to merge 6 commits into
mainfrom
copilot/sub-pr-54-again
Draft

test: unit tests for get_package and get_partitions methods#56
cmatsuoka with Copilot wants to merge 6 commits into
mainfrom
copilot/sub-pr-54-again

Conversation

Copilot AI commented Nov 27, 2025

Copy link
Copy Markdown
Contributor
  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run make lint && make test?

Addresses review feedback from #54 requesting unit test coverage for the new get_package and get_partitions methods in project models.

Changes

  • Added tests for Project.get_package():

    • Successful package retrieval
    • Error when no packages defined
    • Error when package not found
  • Added tests for PackagesProject.get_partitions():

    • Returns ["default"] when no packages defined
    • Returns ["default", "package/<name>", ...] when packages defined
def test_get_package_success(default_project):
    package = default_project.get_package("package-1")
    assert package.version == "2.0"

def test_get_partitions_with_packages():
    packages_project = project.PackagesProject(
        packages={"pkg-a": Package(version="1.0"), "pkg-b": Package(version="2.0")}
    )
    result = packages_project.get_partitions()
    assert result == ["default", "package/pkg-a", "package/pkg-b"]

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

cmatsuoka and others added 3 commits November 27, 2025 19:47
Iterate through all packages declared in the project file and
create a deb file for each file, adding proper control metadata
to each one.

Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Copilot AI and others added 2 commits November 27, 2025 23:00
Co-authored-by: cmatsuoka <317355+cmatsuoka@users.noreply.github.com>
Co-authored-by: cmatsuoka <317355+cmatsuoka@users.noreply.github.com>
Copilot AI changed the title [WIP] Address feedback from review on creating debs for all declared packages Add unit tests for get_package and get_partitions methods Nov 27, 2025
Copilot AI requested a review from cmatsuoka November 27, 2025 23:03
@cmatsuoka cmatsuoka changed the title Add unit tests for get_package and get_partitions methods test: unit tests for get_package and get_partitions methods Nov 28, 2025
@cmatsuoka
cmatsuoka requested a review from lengau November 28, 2025 14:14
@cmatsuoka

cmatsuoka commented Nov 28, 2025

Copy link
Copy Markdown
Contributor

We can land this directly on main after landing #54

Copilot AI left a comment

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.

Pull request overview

This PR adds unit test coverage for two methods in project models: Project.get_package() and PackagesProject.get_partitions(), addressing review feedback from a previous PR (#54).

  • Tests for successful package retrieval and error cases (no packages, package not found)
  • Tests for partition generation with and without packages defined

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/unit/models/test_project_model.py Outdated
Comment on lines +131 to +135
assert result is not None
assert "default" in result
assert "package/pkg-a" in result
assert "package/pkg-b" in result
assert len(result) == 3

Copilot AI Nov 28, 2025

Copy link

Choose a reason for hiding this comment

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

The test uses multiple assertions to verify the result instead of a single comprehensive assertion. Since get_partitions() returns a deterministic list, use a single assertion comparing the full result. Replace lines 131-135 with: assert result == [\"default\", \"package/pkg-a\", \"package/pkg-b\"]. This is clearer, more maintainable, and catches ordering issues.

Suggested change
assert result is not None
assert "default" in result
assert "package/pkg-a" in result
assert "package/pkg-b" in result
assert len(result) == 3
assert result == ["default", "package/pkg-a", "package/pkg-b"]

Copilot uses AI. Check for mistakes.
Comment thread tests/unit/models/test_project_model.py Outdated
}
)
result = packages_project.get_partitions()
assert result is not None

Copilot AI Nov 28, 2025

Copy link

Choose a reason for hiding this comment

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

This assertion is redundant. The get_partitions() method always returns a list (either [\"default\"] or a list with packages), never None. The return type annotation list[str] | None appears to allow None, but examining the implementation shows it always returns a list. This check can be removed.

Suggested change
assert result is not None

Copilot uses AI. Check for mistakes.
Comment thread tests/unit/models/test_project_model.py Outdated
def test_get_package_success(default_project):
"""Test that get_package returns the correct package."""
package = default_project.get_package("package-1")
assert package is not None

Copilot AI Nov 28, 2025

Copy link

Choose a reason for hiding this comment

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

This assertion is redundant. The get_package() method either returns a Package or raises an exception, so it can never return None. Line 91 already verifies the package content by checking the version. Remove this assertion.

Suggested change
assert package is not None

Copilot uses AI. Check for mistakes.
Base automatically changed from work/DEBCRAFT-21-Create-multiple-deb-packages to main December 4, 2025 14:41
@lengau

lengau commented Dec 6, 2025

Copy link
Copy Markdown
Contributor

@copilot rebase this on main.

Co-authored-by: lengau <4305943+lengau@users.noreply.github.com>

Copilot AI commented Dec 6, 2025

Copy link
Copy Markdown
Contributor Author

@copilot rebase this on main.

I'm unable to perform git rebase operations as I don't have access to force push. Could you please rebase this branch on main?

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.

4 participants