Skip to content

fix(setup): implement isSetup() to check for matched.json marker#513

Open
karthik120710 wants to merge 2 commits into
open-telemetry:mainfrom
karthik120710:issue#512
Open

fix(setup): implement isSetup() to check for matched.json marker#513
karthik120710 wants to merge 2 commits into
open-telemetry:mainfrom
karthik120710:issue#512

Conversation

@karthik120710
Copy link
Copy Markdown

@karthik120710 karthik120710 commented May 17, 2026

Description:

Summary

Fixes the unimplemented isSetup() guard in tool/internal/setup/setup.go.

Resolves #

Changes

  • Implemented isSetup() to check for the presence of matched.json (written by store() at the end of a successful Setup run) instead of always
    returning false
  • Added unit tests for both the "not set up" and "already set up" cases

Why

Without this fix, the guard at Setup() line 149 would never short-circuit, causing setup to re-run on every build once the guard is wired in —
resulting in redundant re-initialization.

Test Plan

  • go test ./tool/internal/setup/... -run TestIsSetup passes
  • Full setup package tests pass: go test ./tool/internal/setup/...

Notes from the guidelines:

  • No changelog entry needed — this is a bug fix for an internal/unshipped guard with no user-facing behavior change. You can prefix the title with
    [chore] if reviewers flag it, or ask a maintainer to add the Skip Changelog label.
  • The title follows the [component] description format required by the guidelines.
    isSetup() is unimplemented and hard-codes false #512

Implement the isSetup function to check for the existence of the matched-rules file, indicating whether the setup has been completed. Add unit tests for isSetup to verify its behavior when the matched.json file is present and absent.

Signed-off-by: Karthik Rajan <karthikrajanmr@gmail.com>
@karthik120710 karthik120710 requested a review from a team as a code owner May 17, 2026 17:57
@github-actions
Copy link
Copy Markdown

The title of this pull request does not match the conventional commits format.
Please update the title, as apprioriate.

Refer to the CONTRIBUTING.md file for more information.

@karthik120710 karthik120710 changed the title test(setup): add isSetup function and corresponding tests [setup] implement isSetup() to check for matched.json marker May 17, 2026
@github-actions
Copy link
Copy Markdown

The title of this pull request does not match the conventional commits format.
Please update the title, as apprioriate.

Refer to the CONTRIBUTING.md file for more information.

@karthik120710 karthik120710 changed the title [setup] implement isSetup() to check for matched.json marker fix(setup): implement isSetup() to check for matched.json marker May 17, 2026
@github-actions github-actions Bot added the scope:fix A bug that is being fixed label May 17, 2026
@karthik120710
Copy link
Copy Markdown
Author

karthik120710 commented May 17, 2026

Hi @txabman42 @kakkoyun can u review my pr

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.88%. Comparing base (e88e6f5) to head (dbc0275).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##           main     #513       +/-   ##
=========================================
+ Coverage      0   62.88%   +62.88%     
=========================================
  Files         0       62       +62     
  Lines         0     4766     +4766     
=========================================
+ Hits          0     2997     +2997     
- Misses        0     1525     +1525     
- Partials      0      244      +244     
Flag Coverage Δ
tool 62.88% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@txabman42 txabman42 left a comment

Choose a reason for hiding this comment

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

Checking the file existence is not enough. The setup file might be related to a previous configuration, making not possible to do the setup again.

This problem is being discussed in this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope:fix A bug that is being fixed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants