Skip to content

Commit 1d2c378

Browse files
committed
loader: expand test coverage and document selection ordering
Follow-up to #875. Adds tests for coverage gaps identified during review and a comment at the call site documenting why WithServicesEnabled must precede WithSelectedServices. - Test that selecting a profile-disabled service re-activates it, locking in the two-step contract. - Test that WithoutUnnecessaryResources prunes secrets, configs and models (only networks/volumes were exercised before). - Test that WithoutUnnecessaryResources works standalone, without WithSelectedServices. - Comment the WithServicesEnabled-before-WithSelectedServices ordering requirement in ModelToProject. Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
1 parent 9b9620a commit 1d2c378

2 files changed

Lines changed: 118 additions & 0 deletions

File tree

loader/loader.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,9 @@ func ModelToProject(dict map[string]interface{}, opts *Options, configDetails ty
630630
}
631631

632632
if len(opts.SelectedServices) > 0 {
633+
// WithServicesEnabled must precede WithSelectedServices: the latter walks
634+
// only active services, so any selected service currently sitting in
635+
// DisabledServices (e.g. gated by a profile) would otherwise be invisible.
633636
project, err = project.WithServicesEnabled(opts.SelectedServices...)
634637
if err != nil {
635638
return nil, err

loader/loader_test.go

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2942,6 +2942,31 @@ services:
29422942
assert.Assert(t, hasAPI, "api dependency should be kept")
29432943
assert.Assert(t, !hasWorker, "worker should have been filtered out")
29442944
})
2945+
2946+
t.Run("re-activates a profile-disabled service when selected", func(t *testing.T) {
2947+
// Without any profile active, "debug" lands in DisabledServices after
2948+
// WithProfiles. WithSelectedServices alone would never see it because
2949+
// it walks only active services; the loader's WithServicesEnabled step
2950+
// is what brings the service back before the filter applies.
2951+
yamlWithProfile := `
2952+
name: test
2953+
services:
2954+
web:
2955+
image: nginx
2956+
debug:
2957+
image: alpine
2958+
profiles:
2959+
- debug
2960+
`
2961+
p, err := LoadWithContext(context.Background(), buildConfigDetails(yamlWithProfile, nil),
2962+
WithSelectedServices([]string{"debug"}),
2963+
)
2964+
assert.NilError(t, err)
2965+
_, hasDebug := p.Services["debug"]
2966+
_, hasWeb := p.Services["web"]
2967+
assert.Assert(t, hasDebug, "debug should be re-activated via selection")
2968+
assert.Assert(t, !hasWeb, "web should have been filtered out")
2969+
})
29452970
}
29462971

29472972
// TestWithSelectedServicesOption_SkipsMissingEnvFile ensures that a `env_file`
@@ -3027,3 +3052,93 @@ volumes:
30273052
assert.Assert(t, !hasTmpVol, "tmp volume referenced only by worker should be pruned")
30283053
})
30293054
}
3055+
3056+
// TestWithoutUnnecessaryResourcesOption_AllResourceKinds covers the secret,
3057+
// config and model resource kinds that WithoutUnnecessaryResources also prunes
3058+
// but that TestWithoutUnnecessaryResourcesOption does not exercise.
3059+
func TestWithoutUnnecessaryResourcesOption_AllResourceKinds(t *testing.T) {
3060+
yaml := `
3061+
name: test
3062+
services:
3063+
web:
3064+
image: nginx
3065+
secrets:
3066+
- used_secret
3067+
configs:
3068+
- used_config
3069+
models:
3070+
- used_model
3071+
worker:
3072+
image: busybox
3073+
secrets:
3074+
- unused_secret
3075+
configs:
3076+
- unused_config
3077+
models:
3078+
- unused_model
3079+
secrets:
3080+
used_secret:
3081+
external: true
3082+
unused_secret:
3083+
external: true
3084+
configs:
3085+
used_config:
3086+
external: true
3087+
unused_config:
3088+
external: true
3089+
models:
3090+
used_model:
3091+
model: ai/used-model
3092+
unused_model:
3093+
model: ai/unused-model
3094+
`
3095+
p, err := LoadWithContext(context.Background(), buildConfigDetails(yaml, nil),
3096+
WithSelectedServices([]string{"web"}),
3097+
WithoutUnnecessaryResources,
3098+
)
3099+
assert.NilError(t, err)
3100+
3101+
_, hasUsedSecret := p.Secrets["used_secret"]
3102+
_, hasUnusedSecret := p.Secrets["unused_secret"]
3103+
assert.Assert(t, hasUsedSecret, "secret referenced by web should remain")
3104+
assert.Assert(t, !hasUnusedSecret, "secret referenced only by worker should be pruned")
3105+
3106+
_, hasUsedConfig := p.Configs["used_config"]
3107+
_, hasUnusedConfig := p.Configs["unused_config"]
3108+
assert.Assert(t, hasUsedConfig, "config referenced by web should remain")
3109+
assert.Assert(t, !hasUnusedConfig, "config referenced only by worker should be pruned")
3110+
3111+
_, hasUsedModel := p.Models["used_model"]
3112+
_, hasUnusedModel := p.Models["unused_model"]
3113+
assert.Assert(t, hasUsedModel, "model referenced by web should remain")
3114+
assert.Assert(t, !hasUnusedModel, "model referenced only by worker should be pruned")
3115+
}
3116+
3117+
// TestWithoutUnnecessaryResourcesOption_NoSelection verifies that the option
3118+
// also prunes resources unreferenced by any service when used standalone
3119+
// (without WithSelectedServices).
3120+
func TestWithoutUnnecessaryResourcesOption_NoSelection(t *testing.T) {
3121+
yaml := `
3122+
name: test
3123+
services:
3124+
web:
3125+
image: nginx
3126+
networks:
3127+
- frontnet
3128+
networks:
3129+
frontnet:
3130+
orphan_net:
3131+
volumes:
3132+
orphan_vol:
3133+
`
3134+
p, err := LoadWithContext(context.Background(), buildConfigDetails(yaml, nil),
3135+
WithoutUnnecessaryResources,
3136+
)
3137+
assert.NilError(t, err)
3138+
_, hasFrontNet := p.Networks["frontnet"]
3139+
_, hasOrphanNet := p.Networks["orphan_net"]
3140+
_, hasOrphanVol := p.Volumes["orphan_vol"]
3141+
assert.Assert(t, hasFrontNet, "network referenced by web should remain")
3142+
assert.Assert(t, !hasOrphanNet, "unreferenced network should be pruned")
3143+
assert.Assert(t, !hasOrphanVol, "unreferenced volume should be pruned")
3144+
}

0 commit comments

Comments
 (0)