Skip to content

Commit bc17d48

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 7c70f41 commit bc17d48

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
@@ -2947,6 +2947,31 @@ services:
29472947
assert.Assert(t, hasAPI, "api dependency should be kept")
29482948
assert.Assert(t, !hasWorker, "worker should have been filtered out")
29492949
})
2950+
2951+
t.Run("re-activates a profile-disabled service when selected", func(t *testing.T) {
2952+
// Without any profile active, "debug" lands in DisabledServices after
2953+
// WithProfiles. WithSelectedServices alone would never see it because
2954+
// it walks only active services; the loader's WithServicesEnabled step
2955+
// is what brings the service back before the filter applies.
2956+
yamlWithProfile := `
2957+
name: test
2958+
services:
2959+
web:
2960+
image: nginx
2961+
debug:
2962+
image: alpine
2963+
profiles:
2964+
- debug
2965+
`
2966+
p, err := LoadWithContext(context.Background(), buildConfigDetails(yamlWithProfile, nil),
2967+
WithSelectedServices([]string{"debug"}),
2968+
)
2969+
assert.NilError(t, err)
2970+
_, hasDebug := p.Services["debug"]
2971+
_, hasWeb := p.Services["web"]
2972+
assert.Assert(t, hasDebug, "debug should be re-activated via selection")
2973+
assert.Assert(t, !hasWeb, "web should have been filtered out")
2974+
})
29502975
}
29512976

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

0 commit comments

Comments
 (0)