Skip to content

clearing backlogs -unit tests for app/cli/tests/discover.py#1270

Merged
Devesh36 merged 2 commits intoTracer-Cloud:mainfrom
Devesh36:discover
May 4, 2026
Merged

clearing backlogs -unit tests for app/cli/tests/discover.py#1270
Devesh36 merged 2 commits intoTracer-Cloud:mainfrom
Devesh36:discover

Conversation

@Devesh36
Copy link
Copy Markdown
Collaborator

@Devesh36 Devesh36 commented May 4, 2026

This pull request adds and improves tests for Makefile target discovery in the tests/cli/test_discover.py file. The main focus is on enhancing test coverage for parsing comments and metadata from Makefiles and refactoring test setup to use temporary files instead of mocks.

Test coverage improvements:

  • Added a new test, test_comment_map_for_makefile_parses_comments_and_resets_buffers, to verify that _comment_map_for_makefile correctly parses comments and associates them with Makefile targets.
  • Added tests to ensure discover_make_targets:
    • Finds targets at the top of the Makefile, even with no preceding newline.
    • Skips targets missing from the Makefile.
    • Correctly applies comments and metadata (such as tags and requirements) to discovered targets.

Test refactoring:

  • Refactored tests to use tmp_path and monkeypatch for creating and patching Makefile paths, replacing the previous use of unittest.mock.

Imports:

  • Added import of _comment_map_for_makefile to support the new tests.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 4, 2026

Greptile Summary

This PR adds test coverage for _comment_map_for_makefile and extends discover_make_targets tests, while refactoring the existing regression test from unittest.mock to tmp_path/monkeypatch for cleaner file-based setup. The new tests correctly exercise comment parsing, target filtering, and metadata application against the real implementation.

Confidence Score: 4/5

Safe to merge; only minor test-quality style issues present, no functional bugs introduced.

All findings are P2 — no logic errors, no security concerns, no broken contracts. The two style issues (over-specified ordering assertion and bare index access) could make future failures less diagnosable but do not affect current correctness.

No files require special attention.

Important Files Changed

Filename Overview
tests/cli/test_discover.py Adds four new tests for _comment_map_for_makefile and discover_make_targets, refactors existing test from unittest.mock to tmp_path/monkeypatch. Logic and assertions are mostly sound; two minor P2 style issues with an over-specified ordering assertion and a bare index access.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["test_comment_map_for_makefile\n_parses_comments_and_resets_buffers"] --> B["_comment_map_for_makefile(path)"]
    B --> C["Parse lines: accumulate\n# comments into buffer"]
    C --> D{"Line type?"}
    D -->|"comment (#)"| E["Append to comment_buffer"]
    D -->|"blank line"| F["Reset comment_buffer"]
    D -->|"target (NAME:)"| G["Store buffer as\ncomment_map[target];\nreset buffer"]
    D -->|"other (recipe/var)"| F

    H["test_discover_make_targets\n_finds_target_at_line_one"] --> I["discover_make_targets()"]
    J["test_discover_make_targets\n_skips_targets_missing_from_makefile"] --> I
    K["test_discover_make_targets\n_applies_comment_and_metadata"] --> I

    I --> L["MAKEFILE_PATH.is_file()?"]
    L -->|No| M["return []"]
    L -->|Yes| N["_comment_map_for_makefile()"]
    N --> O["Iterate _TARGETS_TO_INDEX"]
    O --> P{"Target in\nMakefile text?"}
    P -->|No| Q["skip"]
    P -->|Yes| R["Lookup _TARGET_METADATA\nfor tags/requirements"]
    R --> S["Build TestCatalogItem\nwith comment as description"]
    S --> T["Append to items list"]
Loading

Reviews (1): Last reviewed commit: "clearing backlogs -unit tests for app/cl..." | Re-trigger Greptile

Comment thread tests/cli/test_discover.py Outdated
Comment thread tests/cli/test_discover.py Outdated
@Devesh36 Devesh36 merged commit 1750463 into Tracer-Cloud:main May 4, 2026
10 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

LGTM → Merged. @Devesh36, your work is in. Every commit counts — thank you for this one.


👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome.

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.

1 participant