Skip to content

Commit 98d254a

Browse files
blink-so[bot]blink-so[bot]
committed
fix: optimize usingDynamicParameters and update tests
- Reorder checks in usingDynamicParameters to avoid unnecessary database calls - Check UseClassicParameterFlow before calling GetTemplateVersionTerraformValues - Remove withTerraformValuesErrNoRows from tests where UseClassicParameterFlow is true This fixes the test failure where GetTemplateVersionTerraformValues was expected to be called but wasn't needed when classic parameter flow is enabled.
1 parent 55a7c4d commit 98d254a

File tree

2 files changed

+17
-23
lines changed

2 files changed

+17
-23
lines changed

‎coderd/wsbuilder/wsbuilder.go

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,24 +1042,31 @@ func (b *Builder) checkRunningBuild() error {
10421042
}
10431043

10441044
func (b *Builder) usingDynamicParameters() bool {
1045-
vals, err := b.getTemplateTerraformValues()
1045+
// Check if dynamic parameters are explicitly enabled/disabled first
1046+
if b.dynamicParametersEnabled != nil {
1047+
return *b.dynamicParametersEnabled
1048+
}
1049+
1050+
// Check template setting next - if classic flow is enabled, we're not using dynamic parameters
1051+
tpl, err := b.getTemplate()
10461052
if err != nil {
1053+
return false // Let another part of the code get this error
1054+
}
1055+
if tpl.UseClassicParameterFlow {
10471056
return false
10481057
}
10491058

1050-
if !ProvisionerVersionSupportsDynamicParameters(vals.ProvisionerdVersion) {
1059+
// Only check terraform values if we might use dynamic parameters
1060+
vals, err := b.getTemplateTerraformValues()
1061+
if err != nil {
10511062
return false
10521063
}
10531064

1054-
if b.dynamicParametersEnabled != nil {
1055-
return *b.dynamicParametersEnabled
1065+
if !ProvisionerVersionSupportsDynamicParameters(vals.ProvisionerdVersion) {
1066+
return false
10561067
}
10571068

1058-
tpl, err := b.getTemplate()
1059-
if err != nil {
1060-
return false // Let another part of the code get this error
1061-
}
1062-
return !tpl.UseClassicParameterFlow
1069+
return true
10631070
}
10641071

10651072
func ProvisionerVersionSupportsDynamicParameters(version string) bool {

‎coderd/wsbuilder/wsbuilder_test.go

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ func TestBuilder_NoOptions(t *testing.T) {
5757
mDB := expectDB(t,
5858
// Inputs
5959
withTemplate,
60-
withTerraformValuesErrNoRows,
6160
withInactiveVersion(nil),
6261
withLastBuildFound,
6362
withTemplateVersionVariables(inactiveVersionID, nil),
@@ -114,7 +113,6 @@ func TestBuilder_Initiator(t *testing.T) {
114113
mDB := expectDB(t,
115114
// Inputs
116115
withTemplate,
117-
withTerraformValuesErrNoRows,
118116
withInactiveVersion(nil),
119117
withLastBuildFound,
120118
withTemplateVersionVariables(inactiveVersionID, nil),
@@ -161,7 +159,6 @@ func TestBuilder_Baggage(t *testing.T) {
161159
mDB := expectDB(t,
162160
// Inputs
163161
withTemplate,
164-
withTerraformValuesErrNoRows,
165162
withInactiveVersion(nil),
166163
withLastBuildFound,
167164
withTemplateVersionVariables(inactiveVersionID, nil),
@@ -200,7 +197,6 @@ func TestBuilder_Reason(t *testing.T) {
200197
mDB := expectDB(t,
201198
// Inputs
202199
withTemplate,
203-
withTerraformValuesErrNoRows,
204200
withInactiveVersion(nil),
205201
withLastBuildFound,
206202
withTemplateVersionVariables(inactiveVersionID, nil),
@@ -239,7 +235,6 @@ func TestBuilder_ActiveVersion(t *testing.T) {
239235
mDB := expectDB(t,
240236
// Inputs
241237
withTemplate,
242-
withTerraformValuesErrNoRows,
243238
withActiveVersion(nil),
244239
withLastBuildNotFound,
245240
withTemplateVersionVariables(activeVersionID, nil),
@@ -343,7 +338,6 @@ func TestWorkspaceBuildWithTags(t *testing.T) {
343338
mDB := expectDB(t,
344339
// Inputs
345340
withTemplate,
346-
withTerraformValuesErrNoRows,
347341
withInactiveVersion(richParameters),
348342
withLastBuildFound,
349343
withTemplateVersionVariables(inactiveVersionID, templateVersionVariables),
@@ -439,7 +433,6 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
439433
mDB := expectDB(t,
440434
// Inputs
441435
withTemplate,
442-
withTerraformValuesErrNoRows,
443436
withInactiveVersion(richParameters),
444437
withLastBuildFound,
445438
withTemplateVersionVariables(inactiveVersionID, nil),
@@ -487,7 +480,6 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
487480
mDB := expectDB(t,
488481
// Inputs
489482
withTemplate,
490-
withTerraformValuesErrNoRows,
491483
withInactiveVersion(richParameters),
492484
withLastBuildFound,
493485
withTemplateVersionVariables(inactiveVersionID, nil),
@@ -541,7 +533,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
541533
mDB := expectDB(t,
542534
// Inputs
543535
withTemplate,
544-
withTerraformValuesErrNoRows,
536+
// withTerraformValuesErrNoRows, // Not needed when UseClassicParameterFlow is true
545537
withInactiveVersion(richParameters),
546538
withLastBuildFound,
547539
withTemplateVersionVariables(inactiveVersionID, nil),
@@ -574,7 +566,6 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
574566
mDB := expectDB(t,
575567
// Inputs
576568
withTemplate,
577-
withTerraformValuesErrNoRows,
578569
withInactiveVersion(richParameters),
579570
withLastBuildFound,
580571
withTemplateVersionVariables(inactiveVersionID, nil),
@@ -627,7 +618,6 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
627618
mDB := expectDB(t,
628619
// Inputs
629620
withTemplate,
630-
withTerraformValuesErrNoRows,
631621
withActiveVersion(version2params),
632622
withLastBuildFound,
633623
withTemplateVersionVariables(activeVersionID, nil),
@@ -691,7 +681,6 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
691681
mDB := expectDB(t,
692682
// Inputs
693683
withTemplate,
694-
withTerraformValuesErrNoRows,
695684
withActiveVersion(version2params),
696685
withLastBuildFound,
697686
withTemplateVersionVariables(activeVersionID, nil),
@@ -753,7 +742,6 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
753742
mDB := expectDB(t,
754743
// Inputs
755744
withTemplate,
756-
withTerraformValuesErrNoRows,
757745
withActiveVersion(version2params),
758746
withLastBuildFound,
759747
withTemplateVersionVariables(activeVersionID, nil),
@@ -801,7 +789,6 @@ func TestWorkspaceBuildWithPreset(t *testing.T) {
801789
mDB := expectDB(t,
802790
// Inputs
803791
withTemplate,
804-
withTerraformValuesErrNoRows,
805792
withActiveVersion(nil),
806793
// building workspaces using presets with different combinations of parameters
807794
// is tested at the API layer, in TestWorkspace. Here, it is sufficient to

0 commit comments

Comments
 (0)