Skip to content

Simplify TestIsActivationJobNeeded to match unconditional source behavior - #53447

Merged
pelikhan merged 2 commits into
mainfrom
copilot/testify-expert-improve-test-quality-again
Aug 17, 2026
Merged

Simplify TestIsActivationJobNeeded to match unconditional source behavior#53447
pelikhan merged 2 commits into
mainfrom
copilot/testify-expert-improve-test-quality-again

Conversation

CopilotAI commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

isActivationJobNeeded() takes no parameters and unconditionally returns true (compiler_jobs.go:23-31), but its test had four subtests that each constructed a WorkflowData, discarded it via var _ = data inside an immediately-invoked closure, and asserted the same constant. The subtests were functionally identical and implied a dependency on WorkflowData that does not exist.

Changes

  • Collapsed the four redundant subtests into a single assertion.
  • Removed the no-op WorkflowData construction and var _ = data closures.
  • Switched from if !cond { t.Errorf(...) } to assert.True, consistent with compiler_jobs_test.go in the same package.
  • Added a doc comment tying the expectation to the behavior documented in the source.

Before:

t.Run("if_condition_present", func(t*testing.T) {
data:=&WorkflowData{If: "if: github.ref == 'refs/heads/main'"}
if!func() bool {
var_=datareturncompiler.isActivationJobNeeded()
}() {
t.Errorf("Expected isActivationJobNeeded to be true when If condition is specified")
}
})

After:

// TestIsActivationJobNeeded verifies that isActivationJobNeeded always returns true.// As documented in compiler_jobs.go, the activation job is unconditionally required to// perform the timestamp check, and it also handles command configuration, text output,// runtime If conditions, and consolidated permission checks.funcTestIsActivationJobNeeded(t*testing.T) {
compiler:=NewCompiler()
assert.True(t, compiler.isActivationJobNeeded(), "activation job is always needed")
}

referencesCustomJobOutputs and jobDependsOnPreActivation were left alone — already covered in compiler_jobs_test.go and custom_job_condition_test.go, with no duplication drift observed.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
CopilotAI changed the title [WIP] Improve test quality in is_task_job_needed_test.goSimplify TestIsActivationJobNeeded to match unconditional source behaviorAug 17, 2026
CopilotAI requested a review from pelikhanAugust 17, 2026 18:30
@pelikhan
pelikhan marked this pull request as ready for review August 17, 2026 18:50
CopilotAI balanced review requested due to automatic review settings August 17, 2026 18:50
@pelikhan
pelikhan merged commit d84acf6 into mainAug 17, 2026
@pelikhan
pelikhan deleted the copilot/testify-expert-improve-test-quality-again branch August 17, 2026 18:50

CopilotAI 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

Simplifies activation-job testing to reflect its unconditional behavior.

Changes:

  • Replaces four redundant subtests with one assert.True.
  • Removes misleading unused WorkflowData setup.
  • Documents why the activation job is always required.
Show a summary per file
FileDescription
pkg/workflow/is_task_job_needed_test.goStreamlines and clarifies the activation-job test.

Review details

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.87.1

Sign up for freeto 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.

[testify-expert] Improve Test Quality: pkg/workflow/is_task_job_needed_test.go

3 participants

@pelikhan