Skip to content

Fix compile error in setup experience script test - #49812

Closed
raju249 wants to merge 1 commit into
mainfrom
fix-setup-experience-mock-signature
Closed

Fix compile error in setup experience script test#49812
raju249 wants to merge 1 commit into
mainfrom
fix-setup-experience-mock-signature

Conversation

@raju249

@raju249 raju249 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Related issue: N/A — fixes a compile break on main.

Description

main currently fails to compile the ee/server/service test package, which turns the lint, lint-incremental, and test-go (main) jobs red on all open PRs.

The SetSetupExperienceScript datastore method was changed to return (changed bool, err error) (commit fbccb8c). The interface, the generated mock, and two of the three mock assignments in setup_experience_test.go were updated, but the assignment on line 350 was missed:

ee/server/service/setup_experience_test.go:350:36: cannot use func(ctx context.Context, script *fleet.Script) error {…}
as mock.SetSetupExperienceScriptFunc value in assignment (typecheck)

This updates that remaining assignment to the new signature, matching lines 252 and 329.

Testing

  • go vet ./ee/server/service/ passes.
  • Confirmed no other SetSetupExperienceScriptFunc assignments use the old signature.

Checklist for submitter

  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.

Testing

  • Added/updated automated tests

Summary by CodeRabbit

  • Tests
    • Updated setup experience error-handling coverage to reflect the latest validation behavior.
    • Confirmed infrastructure connection errors continue to propagate correctly instead of being classified as invalid-argument errors.

setup_experience_test.go:350 still used the old SetSetupExperienceScript
mock signature after it changed to return (bool, error), which broke
compilation of the ee/server/service test package on main and failed the
lint and test-go (main) jobs on all open PRs.
@raju249
raju249 requested a review from a team as a code owner July 23, 2026 12:09
@raju249

raju249 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Closing — another team member already has a fix for this main breakage in flight. Closing to avoid a duplicate PR.

@raju249 raju249 closed this Jul 23, 2026
@raju249
raju249 deleted the fix-setup-experience-mock-signature branch July 23, 2026 12:11
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0097f5bd-2551-43c1-af8f-37f5aec395e8

📥 Commits

Reviewing files that changed from the base of the PR and between 6fc48f4 and 07d74d4.

📒 Files selected for processing (1)
  • ee/server/service/setup_experience_test.go

Walkthrough

Updated the setup experience test’s SetSetupExperienceScriptFunc mock to return (false, nil) instead of only an error, matching the updated function signature while retaining the infrastructure validation error propagation coverage.

Possibly related PRs

  • fleetdm/fleet#49586: Changes setup experience infrastructure error propagation tied to this mock signature.
  • fleetdm/fleet#49809: Updates the same test mock for the (bool, error) return signature.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-setup-experience-mock-signature

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.

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.90%. Comparing base (fbccb8c) to head (07d74d4).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #49812      +/-   ##
==========================================
+ Coverage   67.44%   67.90%   +0.45%     
==========================================
  Files        3903     3903              
  Lines      249206   249206              
  Branches    13301    13301              
==========================================
+ Hits       168079   169218    +1139     
+ Misses      65863    64749    -1114     
+ Partials    15264    15239      -25     
Flag Coverage Δ
backend 69.30% <ø> (+0.54%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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