Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cli/azd/cmd/figspec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ func TestFigSpec(t *testing.T) {
t.Setenv("AZD_CONFIG_DIR", configDir)
t.Setenv("AZURE_DEV_COLLECT_TELEMETRY", "no")
t.Setenv("AZD_SKIP_FIRST_RUN", "true")
t.Setenv("AZD_ALPHA_ENABLE_TOOL", "true")

cli := azdcli.NewCLI(t)

Expand Down
16 changes: 3 additions & 13 deletions cli/azd/cmd/middleware/tool_first_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/azure/azure-dev/cli/azd/internal/tracing"
"github.com/azure/azure-dev/cli/azd/internal/tracing/fields"
"github.com/azure/azure-dev/cli/azd/internal/tracing/resource"
"github.com/azure/azure-dev/cli/azd/pkg/alpha"
"github.com/azure/azure-dev/cli/azd/pkg/config"
"github.com/azure/azure-dev/cli/azd/pkg/input"
"github.com/azure/azure-dev/cli/azd/pkg/output"
Expand Down Expand Up @@ -52,10 +51,9 @@ const envKeySkipFirstRun = "AZD_SKIP_FIRST_RUN"
// Keep them separate so dashboards can distinguish "user told us not to ask"
// from "we had no choice but to skip".
//
// Skip reasons are *not* emitted for the alpha-disabled and child-action
// paths: when the alpha feature is off the user has no opportunity to opt in,
// and child actions (e.g. workflow steps) inherit the parent's first-run
// state. Neither contributes useful signal to first-run adoption analysis.
// A skip reason is *not* emitted for the child-action path: child actions
// (e.g. workflow steps) inherit the parent's first-run state and do not
// contribute useful signal to first-run adoption analysis.
const (
skipReasonEnvVar = "env_var"
skipReasonNoPrompt = "no_prompt"
Expand All @@ -80,7 +78,6 @@ const (
// installed Azure development tools and optionally offers to
// install any missing recommended tools.
type ToolFirstRunMiddleware struct {
alphaManager *alpha.FeatureManager
configManager config.UserConfigManager
console input.Console
manager *tool.Manager
Expand All @@ -89,14 +86,12 @@ type ToolFirstRunMiddleware struct {

// NewToolFirstRunMiddleware creates a new [ToolFirstRunMiddleware].
func NewToolFirstRunMiddleware(
alphaManager *alpha.FeatureManager,
configManager config.UserConfigManager,
console input.Console,
manager *tool.Manager,
options *internal.GlobalCommandOptions,
) Middleware {
return &ToolFirstRunMiddleware{
alphaManager: alphaManager,
configManager: configManager,
console: console,
manager: manager,
Expand All @@ -109,11 +104,6 @@ func NewToolFirstRunMiddleware(
// always delegates to nextFn so the user's intended command is
// never blocked.
func (m *ToolFirstRunMiddleware) Run(ctx context.Context, nextFn NextFn) (*actions.ActionResult, error) {
// Skip when the tool alpha feature is not enabled.
if !m.alphaManager.IsEnabled(tool.FeatureAlphaTool) {
return nextFn(ctx)
}

// Skip for child actions (e.g. workflow steps).
if IsChildAction(ctx) {
return nextFn(ctx)
Expand Down
123 changes: 29 additions & 94 deletions cli/azd/cmd/middleware/tool_first_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/azure/azure-dev/cli/azd/internal"
"github.com/azure/azure-dev/cli/azd/internal/tracing"
"github.com/azure/azure-dev/cli/azd/internal/tracing/fields"
"github.com/azure/azure-dev/cli/azd/pkg/alpha"
"github.com/azure/azure-dev/cli/azd/pkg/config"
"github.com/azure/azure-dev/cli/azd/pkg/tool"
"github.com/azure/azure-dev/cli/azd/test/mocks/mockinput"
Expand Down Expand Up @@ -74,7 +73,6 @@ func passthroughNext(called *bool) NextFn {
func TestToolFirstRunMiddleware_SkipConditions(t *testing.T) {
// These tests modify env vars via t.Setenv so they cannot be
// parallel with each other or with tests that read the same vars.
t.Setenv("AZD_ALPHA_ENABLE_TOOL", "true")

tests := []struct {
name string
Expand Down Expand Up @@ -158,7 +156,6 @@ func TestToolFirstRunMiddleware_SkipConditions(t *testing.T) {
ucm := &mockUserConfigManager{cfg: cfg}

m := &ToolFirstRunMiddleware{
alphaManager: alpha.NewFeaturesManagerWithConfig(config.NewEmptyConfig()),
configManager: ucm,
console: console,
manager: nil, // not needed for skip paths
Expand Down Expand Up @@ -195,15 +192,13 @@ func TestToolFirstRunMiddleware_TriggersWhenNoSkip(t *testing.T) {
clearCIVars(t)
t.Setenv(envKeySkipFirstRun, "")
os.Unsetenv(envKeySkipFirstRun)
t.Setenv("AZD_ALPHA_ENABLE_TOOL", "true")

console := mockinput.NewMockConsole()
cfg := config.NewEmptyConfig()
opts := &internal.GlobalCommandOptions{}
ucm := &mockUserConfigManager{cfg: cfg}

m := &ToolFirstRunMiddleware{
alphaManager: alpha.NewFeaturesManagerWithConfig(config.NewEmptyConfig()),
configManager: ucm,
console: console,
manager: nil, // runFirstRunExperience will fail at prompt
Expand All @@ -226,44 +221,10 @@ func TestToolFirstRunMiddleware_TriggersWhenNoSkip(t *testing.T) {
"triggered first-run experience should produce console output")
}

// TestToolFirstRunMiddleware_SkipsWhenAlphaDisabled verifies that the
// middleware is a no-op when the tool alpha feature is not enabled.
func TestToolFirstRunMiddleware_SkipsWhenAlphaDisabled(t *testing.T) {
clearCIVars(t)
t.Setenv(envKeySkipFirstRun, "")
os.Unsetenv(envKeySkipFirstRun)
t.Setenv("AZD_ALPHA_ENABLE_TOOL", "false")

console := mockinput.NewMockConsole()
cfg := config.NewEmptyConfig()
opts := &internal.GlobalCommandOptions{}
ucm := &mockUserConfigManager{cfg: cfg}

m := &ToolFirstRunMiddleware{
alphaManager: alpha.NewFeaturesManagerWithConfig(config.NewEmptyConfig()),
configManager: ucm,
console: console,
manager: nil,
options: opts,
}

nextCalled := false
result, err := m.Run(t.Context(), passthroughNext(&nextCalled))

require.NoError(t, err)
require.NotNil(t, result)
assert.True(t, nextCalled,
"nextFn must always be called even when alpha is disabled")
assert.Empty(t, console.Output(),
"alpha-disabled should skip the first-run experience entirely")
}

// TestToolFirstRunMiddleware_EmitsSkipReason verifies that each documented
// skip path sets the tool.firstrun.skip_reason usage attribute to the
// expected value.
func TestToolFirstRunMiddleware_EmitsSkipReason(t *testing.T) {
t.Setenv("AZD_ALPHA_ENABLE_TOOL", "true")

tests := []struct {
name string
setup func(t *testing.T, console *mockinput.MockConsole, cfg config.Config, opts *internal.GlobalCommandOptions)
Expand Down Expand Up @@ -323,7 +284,6 @@ func TestToolFirstRunMiddleware_EmitsSkipReason(t *testing.T) {

ucm := &mockUserConfigManager{cfg: cfg}
m := &ToolFirstRunMiddleware{
alphaManager: alpha.NewFeaturesManagerWithConfig(config.NewEmptyConfig()),
configManager: ucm,
console: console,
manager: nil,
Expand All @@ -342,65 +302,42 @@ func TestToolFirstRunMiddleware_EmitsSkipReason(t *testing.T) {
}
}

// TestToolFirstRunMiddleware_NoSkipReasonForSilentPaths verifies that the
// alpha-disabled and child-action skip paths do NOT emit
// tool.firstrun.skip_reason (these paths are intentionally silent because the
// user has no opportunity to opt in / out).
func TestToolFirstRunMiddleware_NoSkipReasonForSilentPaths(t *testing.T) {
// TestToolFirstRunMiddleware_NoSkipReasonForChildAction verifies that the
// child-action skip path does NOT emit tool.firstrun.skip_reason — child
// actions inherit their parent's first-run state and shouldn't contribute
// to first-run adoption analysis.
func TestToolFirstRunMiddleware_NoSkipReasonForChildAction(t *testing.T) {
clearCIVars(t)
t.Setenv(envKeySkipFirstRun, "")
os.Unsetenv(envKeySkipFirstRun)

cases := []struct {
name string
alphaOn bool
childCtx bool
}{
{name: "alpha_disabled", alphaOn: false},
{name: "child_action", alphaOn: true, childCtx: true},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if tc.alphaOn {
t.Setenv("AZD_ALPHA_ENABLE_TOOL", "true")
} else {
t.Setenv("AZD_ALPHA_ENABLE_TOOL", "false")
}

tracing.ResetUsageAttributesForTest()

console := mockinput.NewMockConsole()
cfg := config.NewEmptyConfig()
opts := &internal.GlobalCommandOptions{}
ucm := &mockUserConfigManager{cfg: cfg}
tracing.ResetUsageAttributesForTest()

m := &ToolFirstRunMiddleware{
alphaManager: alpha.NewFeaturesManagerWithConfig(config.NewEmptyConfig()),
configManager: ucm,
console: console,
manager: nil,
options: opts,
}
console := mockinput.NewMockConsole()
cfg := config.NewEmptyConfig()
opts := &internal.GlobalCommandOptions{}
ucm := &mockUserConfigManager{cfg: cfg}

ctx := t.Context()
if tc.childCtx {
ctx = WithChildAction(ctx)
}
m := &ToolFirstRunMiddleware{
configManager: ucm,
console: console,
manager: nil,
options: opts,
}

nextCalled := false
_, err := m.Run(ctx, passthroughNext(&nextCalled))
require.NoError(t, err)
assert.True(t, nextCalled)
ctx := WithChildAction(t.Context())

// Silent skip paths (alpha-disabled / child-action) must not
// emit tool.firstrun.skip_reason at all — the user had no
// opportunity to opt in/out, so there's nothing to record.
_, ok := lookupUsageAttr(string(fields.ToolFirstRunSkipReasonKey.Key))
assert.False(t, ok,
"silent skip path must not emit tool.firstrun.skip_reason")
})
}
nextCalled := false
_, err := m.Run(ctx, passthroughNext(&nextCalled))
require.NoError(t, err)
assert.True(t, nextCalled)

// Child-action skip path must not emit tool.firstrun.skip_reason —
// child actions inherit the parent's first-run state, so there's
// nothing meaningful to record.
_, ok := lookupUsageAttr(string(fields.ToolFirstRunSkipReasonKey.Key))
assert.False(t, ok,
"child-action skip path must not emit tool.firstrun.skip_reason")
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -515,17 +452,15 @@ func TestToolFirstRunMiddleware_DisplayToolStatuses(t *testing.T) {
// every dependency on the returned middleware. This guards against future
// refactors that silently drop a field assignment.
func TestNewToolFirstRunMiddleware(t *testing.T) {
alphaMgr := alpha.NewFeaturesManagerWithConfig(config.NewEmptyConfig())
console := mockinput.NewMockConsole()
ucm := &mockUserConfigManager{cfg: config.NewEmptyConfig()}
opts := &internal.GlobalCommandOptions{}

mw := NewToolFirstRunMiddleware(alphaMgr, ucm, console, nil, opts)
mw := NewToolFirstRunMiddleware(ucm, console, nil, opts)
require.NotNil(t, mw)

concrete, ok := mw.(*ToolFirstRunMiddleware)
require.True(t, ok, "NewToolFirstRunMiddleware must return a *ToolFirstRunMiddleware")
assert.Same(t, alphaMgr, concrete.alphaManager)
assert.Same(t, ucm, concrete.configManager)
assert.Same(t, console, concrete.console)
assert.Nil(t, concrete.manager)
Expand Down
9 changes: 0 additions & 9 deletions cli/azd/cmd/middleware/tool_update_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/azure/azure-dev/cli/azd/cmd/actions"
"github.com/azure/azure-dev/cli/azd/internal"
"github.com/azure/azure-dev/cli/azd/internal/tracing/resource"
"github.com/azure/azure-dev/cli/azd/pkg/alpha"
"github.com/azure/azure-dev/cli/azd/pkg/input"
"github.com/azure/azure-dev/cli/azd/pkg/output"
"github.com/azure/azure-dev/cli/azd/pkg/tool"
Expand All @@ -26,7 +25,6 @@ import (
// interactive and the current command is not a tool-management
// subcommand.
type ToolUpdateCheckMiddleware struct {
alphaManager *alpha.FeatureManager
manager *tool.Manager
console input.Console
options *Options
Expand All @@ -36,14 +34,12 @@ type ToolUpdateCheckMiddleware struct {
// NewToolUpdateCheckMiddleware creates a new [ToolUpdateCheckMiddleware].
// All dependencies are resolved by the IoC container.
func NewToolUpdateCheckMiddleware(
alphaManager *alpha.FeatureManager,
manager *tool.Manager,
console input.Console,
options *Options,
globalOptions *internal.GlobalCommandOptions,
) Middleware {
return &ToolUpdateCheckMiddleware{
alphaManager: alphaManager,
manager: manager,
console: console,
options: options,
Expand All @@ -56,11 +52,6 @@ func NewToolUpdateCheckMiddleware(
// completes it triggers a background update check when the configured
// check interval has elapsed.
func (m *ToolUpdateCheckMiddleware) Run(ctx context.Context, nextFn NextFn) (*actions.ActionResult, error) {
// Skip when the tool alpha feature is not enabled.
if !m.alphaManager.IsEnabled(tool.FeatureAlphaTool) {
return nextFn(ctx)
}

// Skip all notification and background-check logic for child
// actions (e.g. workflow steps invoked by a parent command).
if !IsChildAction(ctx) {
Expand Down
Loading
Loading