Skip to content

Conversation

@mikusaq
Copy link
Contributor

@mikusaq mikusaq commented Nov 5, 2025

Add integration test for Meson build system and some wrong defines/options for both CMake and Meson.

Summary by CodeRabbit

  • Tests

    • Expanded test suite with eight new cases covering Meson and CMake builds, including validations for invalid/empty defines, invalid options, wrong types, and a successful Meson debug build.
    • Added multiple new test data package configurations to exercise these scenarios.
  • Chores

    • Simplified test build setup by removing an intermediate external fetch step prior to building the test binary.

@mikusaq mikusaq self-assigned this Nov 5, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 5, 2025

Walkthrough

Removes a go get step from the packager build fixture and adds eight integration tests (test_26–test_33) covering Meson and CMake define/option validation and a successful Meson build, plus several new JSON test package configurations used by those tests.

Changes

Cohort / File(s) Summary
Build fixture
tests/conftest.py
Removed initial go get invocation for bap-builder; the packager_binary fixture now builds with go build directly.
Integration tests
tests/integration_tests/test_build_package.py
Added 8 tests: test_26_meson_build (successful Meson build) and test_27test_33 validating invalid/empty defines/options and wrong define types for Meson/CMake. Tests follow existing prepare/skip/run patterns.
Test data: Meson/CMake cases
tests/test_data/test_packages/test_package_10/c_example_debug.json
tests/test_data/test_packages/test_package_10_28/c_example_debug.json
tests/test_data/test_packages/test_package_10_29/c_example_debug.json
tests/test_data/test_packages/test_package_10_30/c_example_debug.json
tests/test_data/test_packages/test_package_10_31/c_example_debug.json
Added JSON package configs exercising Meson build options and edge cases (invalid/empty define/option names, non-string define values) and a debug build variant.
Test data: zlib CMake cases
tests/test_data/test_packages/test_package_1_27/zlib_release.json
tests/test_data/test_packages/test_package_1_32/zlib_release.json
tests/test_data/test_packages/test_package_1_33/zlib_release.json
Added CMake-based JSON configs for zlib release variants including CMAKE_BUILD_TYPE and other defines, used by new tests.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Integration Test
    participant CLI as packager CLI
    participant Validator as BuildSystem Validator
    participant Builder as Meson/CMake Build

    Test->>CLI: invoke run_packager with package config
    CLI->>Validator: parse build config (defines/options)
    alt invalid/empty/wrong-type
        Validator-->>CLI: return CONTEXT_ERROR
        CLI-->>Test: exit with expected failure
    else valid
        Validator->>Builder: construct build commands
        Builder-->>CLI: build success or failure
        CLI-->>Test: return success or failure code
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Areas to focus:
    • New test logic in tests/integration_tests/test_build_package.py for correct assertions and skips.
    • packager_binary fixture change in tests/conftest.py to ensure build artifacts and paths remain correct.
    • Verify JSON test files' structure matches parser/validation expectations (empty keys, invalid characters, non-string values).

Possibly related PRs

Suggested reviewers

  • koudis

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add integration tests for Meson' accurately describes the main change: adding new integration tests for Meson build system, as evidenced by test_26 through test_33 covering Meson scenarios and invalid parameter handling.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch BAF-1174/meson-tests

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e03f4f and a71372a.

📒 Files selected for processing (6)
  • tests/integration_tests/test_build_package.py (1 hunks)
  • tests/test_data/test_packages/test_package_10/c_example_debug.json (1 hunks)
  • tests/test_data/test_packages/test_package_10_28/c_example_debug.json (1 hunks)
  • tests/test_data/test_packages/test_package_10_29/c_example_debug.json (1 hunks)
  • tests/test_data/test_packages/test_package_10_30/c_example_debug.json (1 hunks)
  • tests/test_data/test_packages/test_package_10_31/c_example_debug.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • tests/test_data/test_packages/test_package_10/c_example_debug.json
  • tests/integration_tests/test_build_package.py
  • tests/test_data/test_packages/test_package_10_29/c_example_debug.json
  • tests/test_data/test_packages/test_package_10_28/c_example_debug.json
  • tests/test_data/test_packages/test_package_10_31/c_example_debug.json
  • tests/test_data/test_packages/test_package_10_30/c_example_debug.json

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (3)
tests/integration_tests/test_build_package.py (3)

756-770: Add image support check for consistency.

For consistency with other tests and to avoid unnecessary CI resource usage, add an image support check at the beginning of the test.

Apply this diff:

 def test_27_cmake_invalid_define(test_image, packager_binary, context, test_repo):
-    """Test building a package with an CMake define with invalid characters."""
+    """Test building a package with a CMake define with invalid characters."""
     package = "test_package_1_27"
     prepare_packages([package])
+    if not does_package_support_image(package, test_image):
+        return
 
     run_packager(
         packager_binary,

844-860: Fix grammar in docstring.

Minor grammar correction needed in the docstring.

Apply this diff:

 def test_32_cmake_empty_define(test_image, packager_binary, context, test_repo):
-    """Test building a package with an CMake define with empty name."""
+    """Test building a package with a CMake define with empty name."""
     package = "test_package_1_32"

862-878: Fix grammar in docstring.

Minor grammar correction needed in the docstring.

Apply this diff:

 def test_33_cmake_wrong_define_type(test_image, packager_binary, context, test_repo):
-    """Test building a package with an CMake define with non-string value."""
+    """Test building a package with a CMake define with non-string value."""
     package = "test_package_1_33"
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9cd69f7 and 6e03f4f.

📒 Files selected for processing (10)
  • tests/conftest.py (0 hunks)
  • tests/integration_tests/test_build_package.py (1 hunks)
  • tests/test_data/test_packages/test_package_10/c_example_debug.json (1 hunks)
  • tests/test_data/test_packages/test_package_10_28/c_example_debug.json (1 hunks)
  • tests/test_data/test_packages/test_package_10_29/c_example_debug.json (1 hunks)
  • tests/test_data/test_packages/test_package_10_30/c_example_debug.json (1 hunks)
  • tests/test_data/test_packages/test_package_10_31/c_example_debug.json (1 hunks)
  • tests/test_data/test_packages/test_package_1_27/zlib_release.json (1 hunks)
  • tests/test_data/test_packages/test_package_1_32/zlib_release.json (1 hunks)
  • tests/test_data/test_packages/test_package_1_33/zlib_release.json (1 hunks)
💤 Files with no reviewable changes (1)
  • tests/conftest.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mikusaq
Repo: bacpack-system/packager PR: 46
File: internal/config/Config.go:134-141
Timestamp: 2025-09-24T10:05:00.810Z
Learning: The BuildSystem.CheckPrerequisites method in internal/build/BuildSystem.go already validates that both CMake and Meson cannot be set simultaneously, so additional validation in Config.fillBuildStructure is not needed.
📚 Learning: 2025-08-29T09:50:44.695Z
Learnt from: mikusaq
Repo: bacpack-system/packager PR: 43
File: tests/test_data/test_packages/test_package_1_18/zlib_release.json:3-6
Timestamp: 2025-08-29T09:50:44.695Z
Learning: For the bacpack-system/packager repository, Git revision tags (like "v1.2.11") are acceptable in test data JSON files and do not need to be replaced with commit SHAs.

Applied to files:

  • tests/test_data/test_packages/test_package_1_33/zlib_release.json
  • tests/test_data/test_packages/test_package_10_29/c_example_debug.json
  • tests/test_data/test_packages/test_package_1_27/zlib_release.json
  • tests/test_data/test_packages/test_package_1_32/zlib_release.json
  • tests/test_data/test_packages/test_package_10/c_example_debug.json
  • tests/test_data/test_packages/test_package_10_31/c_example_debug.json
  • tests/test_data/test_packages/test_package_10_30/c_example_debug.json
📚 Learning: 2025-08-29T09:51:15.354Z
Learnt from: mikusaq
Repo: bacpack-system/packager PR: 43
File: tests/test_data/test_packages/test_package_1_25/zlib_debug.json:4-6
Timestamp: 2025-08-29T09:51:15.354Z
Learning: In the bacpack-system/packager repository, test files with invalid Git revisions like "not_real_version_tag" in tests/test_data/test_packages/test_package_1_25/ are intentional negative test cases designed to test error handling, not actual issues that need fixing.

Applied to files:

  • tests/test_data/test_packages/test_package_1_33/zlib_release.json
  • tests/test_data/test_packages/test_package_10_29/c_example_debug.json
  • tests/test_data/test_packages/test_package_1_32/zlib_release.json
  • tests/integration_tests/test_build_package.py
  • tests/test_data/test_packages/test_package_10_31/c_example_debug.json
  • tests/test_data/test_packages/test_package_10_30/c_example_debug.json
🧬 Code graph analysis (1)
tests/integration_tests/test_build_package.py (4)
tests/conftest.py (4)
  • test_image (45-52)
  • packager_binary (71-79)
  • context (56-57)
  • test_repo (61-62)
tests/test_utils/test_utils.py (3)
  • prepare_packages (79-86)
  • does_package_support_image (165-182)
  • run_packager (193-294)
tests/test_utils/common.py (2)
  • PackagerExpectedResult (20-29)
  • PackagerReturnCode (4-17)
internal/packager_error/PackagerError.go (1)
  • CONTEXT_ERROR (10-10)
🔇 Additional comments (15)
tests/test_data/test_packages/test_package_10/c_example_debug.json (1)

1-32: Valid baseline test configuration.

Clean Meson debug configuration that matches directory structure and appears to serve as a valid baseline case for the integration tests.

tests/test_data/test_packages/test_package_1_32/zlib_release.json (2)

15-15: Package name does not match directory structure.

Line 15 specifies "Name": "test_package_1_33", but the file is located in test_package_1_32/ directory. Verify whether this mismatch is intentional or if the package name should be test_package_1_32.


9-11: Empty define key is intentional negative test case.

The empty string "" key in CMake Defines is an intentional invalid configuration to test error handling for empty define names. This is consistent with the PR objective to test incorrect/wrong defines.

tests/test_data/test_packages/test_package_10_30/c_example_debug.json (1)

1-35: Valid negative test case for empty Meson define.

This configuration intentionally includes an empty "" key in Meson Defines to test error handling for invalid (empty) define names, aligning with the PR objective.

tests/test_data/test_packages/test_package_10_28/c_example_debug.json (1)

1-35: Valid negative test case for invalid Meson define name.

The "INVALID$DEFINE" key intentionally contains a special character to test error handling for malformed define names.

tests/test_data/test_packages/test_package_1_27/zlib_release.json (2)

16-16: Package name does not match directory structure.

Line 16 specifies "Name": "test_package_1_26", but the file is located in test_package_1_27/ directory. Verify whether this mismatch is intentional or if the package name should be test_package_1_27.


9-12: Valid negative test case with mixed valid and invalid CMake defines.

The "INVALID$DEFINE" key intentionally contains a special character to test error handling, while CMAKE_BUILD_TYPE: Release is a valid define. This mixed scenario tests validation logic.

tests/test_data/test_packages/test_package_10_29/c_example_debug.json (1)

1-33: Valid negative test case for invalid Meson option.

The "invalid_option": "option" key tests how the build system handles unrecognized Meson options, while buildtype: debug is valid. This mixed scenario validates option validation logic.

tests/test_data/test_packages/test_package_10_31/c_example_debug.json (1)

1-32: Valid negative test case for empty Meson option key.

The empty "" key in Meson Options tests error handling for invalid (empty) option names.

tests/test_data/test_packages/test_package_1_33/zlib_release.json (2)

16-16: Package name does not match directory structure.

Line 16 specifies "Name": "test_package_1_35", but the file is located in test_package_1_33/ directory. Verify whether this mismatch is intentional or if the package name should be test_package_1_33.


7-13: Valid negative test case with boolean CMake define value.

The "SOME_DEFINE": true intentionally uses a boolean value instead of a string to test type validation, while CMAKE_BUILD_TYPE: Release is valid. This tests whether the build system validates that define values should be strings.

tests/integration_tests/test_build_package.py (4)

772-788: LGTM!

The test correctly validates invalid Meson defines, checks image support, and expects the appropriate error code.


790-806: LGTM!

The test correctly validates invalid Meson options, checks image support, and expects the appropriate error code.


808-824: LGTM!

The test correctly validates empty Meson define names, checks image support, and expects the appropriate error code.


826-842: LGTM!

The test correctly validates empty Meson option names, checks image support, and expects the appropriate error code.

@mikusaq mikusaq mentioned this pull request Dec 10, 2025
1 task
@mikusaq
Copy link
Contributor Author

mikusaq commented Dec 10, 2025

Moved to #49, closing this PR.

@mikusaq mikusaq closed this Dec 10, 2025
mikusaq added a commit that referenced this pull request Dec 29, 2025
Add integration tests for Meson #47 and for local repo. Adds using test Context with simple Packages and Apps only for test purposes instead of external real dependencies, which can be unstable.  Add setup before running tests to ensure consistency - building Packager binary, removing and building Docker images.
@mikusaq mikusaq deleted the BAF-1174/meson-tests branch January 26, 2026 08:26
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