Skip to content

Commit

Permalink
[#674] Add handling of variant parameter authorized values
Browse files Browse the repository at this point in the history
- change param.variant type to string to be consistant with other params handling
- remove config/param_variant_test.go
- add variant.Select(string) to manage variant instance selection
- add UnsupportedVariantError (returned when variant name is not recognized)
- ensure TCR engine exits on error when variant is not recognized
  • Loading branch information
mengdaming committed Oct 1, 2024
1 parent 6bebc08 commit e5eb2ba
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 70 deletions.
46 changes: 0 additions & 46 deletions src/config/param_variant_test.go

This file was deleted.

3 changes: 1 addition & 2 deletions src/config/tcr_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/murex/tcr/settings"
"github.com/murex/tcr/toolchain"
"github.com/murex/tcr/utils"
"github.com/murex/tcr/variant"
"github.com/spf13/cobra"
"github.com/spf13/viper"
"io"
Expand Down Expand Up @@ -221,7 +220,7 @@ func UpdateEngineParams(p *params.Params) {
p.Toolchain = Config.Toolchain.GetValue()
p.PollingPeriod = Config.PollingPeriod.GetValue()
p.AutoPush = Config.AutoPush.GetValue()
p.Variant = variant.Variant(Config.Variant.GetValue())
p.Variant = Config.Variant.GetValue()
p.CommitFailures = Config.CommitFailures.GetValue()
p.VCS = Config.VCS.GetValue()
p.MessageSuffix = Config.MessageSuffix.GetValue()
Expand Down
26 changes: 17 additions & 9 deletions src/engine/tcr.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type (
ToggleAutoPush()
SetAutoPush(flag bool)
SetCommitOnFail(flag bool)
SetVariant(variant variant.Variant)
SetVariant(variant string)
GetCurrentRole() role.Role
RunAsDriver()
RunAsNavigator()
Expand Down Expand Up @@ -94,7 +94,7 @@ type (
// before starting a new one
roleMutex sync.Mutex
commitOnFail bool
variant variant.Variant
variant *variant.Variant
messageSuffix string
// shoot channel is used for handling interruptions coming from the UI
shoot chan bool
Expand All @@ -109,10 +109,6 @@ type (
}
)

func (tcr *TCREngine) SetVariant(variant variant.Variant) {
tcr.variant = variant
}

const traceReporterWaitingTime = 100 * time.Millisecond

const fsWatchRearmDelay = 100 * time.Millisecond
Expand Down Expand Up @@ -200,6 +196,18 @@ func (tcr *TCREngine) SetCommitOnFail(flag bool) {
}
}

// SetVariant sets the TCR variant that will be used by TCR engine
func (tcr *TCREngine) SetVariant(variantName string) {
var err error
tcr.variant, err = variant.Select(variantName)
if err != nil {
var unsupportedVariantError *variant.UnsupportedVariantError
if errors.As(err, &unsupportedVariantError) {
tcr.handleError(err, true, status.ConfigError)
}
}
}

func (tcr *TCREngine) setMobTimerDuration(duration time.Duration) {
if settings.EnableMobTimer {
if tcr.mode.IsMultiRole() {
Expand Down Expand Up @@ -633,14 +641,14 @@ func (tcr *TCREngine) revert(event events.TCREvent) {
}

func (tcr *TCREngine) noFilesRevertedMessage() string {
if tcr.variant == variant.Relaxed {
if *tcr.variant == variant.Relaxed {
return "No file reverted (only test files were updated since last commit)"
}
return "No file reverted"
}

func (tcr *TCREngine) shouldRevertFile(path string) bool {
return tcr.variant == variant.BTCR || tcr.language.IsSrcFile(path)
return *tcr.variant == variant.BTCR || tcr.language.IsSrcFile(path)
}

func (tcr *TCREngine) commitTestBreakingChanges(event events.TCREvent) (err error) {
Expand Down Expand Up @@ -694,7 +702,7 @@ func (tcr *TCREngine) GetSessionInfo() SessionInfo {
VCSSessionSummary: tcr.vcs.SessionSummary(),
GitAutoPush: tcr.vcs.IsAutoPushEnabled(),
CommitOnFail: tcr.commitOnFail,
Variant: string(tcr.variant),
Variant: tcr.variant.Name(),
MessageSuffix: tcr.messageSuffix,
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/engine/tcr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ func Test_variant_specific_reverts(t *testing.T) {
},
)
tcr, vcsFake := initTCREngineWithFakesWithFileDiffs(
params.AParamSet(params.WithVariant(tt.variant)),
params.AParamSet(params.WithVariant(tt.variant.Name())),
nil, nil, nil, tt.fileDiffs)
tcr.revert(*events.ATcrEvent())
sniffer.Stop()
Expand All @@ -363,7 +363,7 @@ func Test_variant_specific_reverts(t *testing.T) {
func Test_introspective_variant(t *testing.T) {
t.Skip("work in progress")
tcr, vcsFake := initTCREngineWithFakesWithFileDiffs(
params.AParamSet(params.WithVariant(variant.Introspective)),
params.AParamSet(params.WithVariant(variant.Introspective.Name())),
nil, nil, nil, vcs.FileDiffs{
vcs.NewFileDiff("fake-src", 1, 1),
vcs.NewFileDiff("fake-test", 1, 1),
Expand Down Expand Up @@ -589,7 +589,7 @@ func Test_set_variant(t *testing.T) {
for _, tt := range testFlags {
t.Run(fmt.Sprintf("Variant %v", tt.variant), func(t *testing.T) {
tcr, _ = initTCREngineWithFakes(nil, nil, nil, nil)
tcr.SetVariant(tt.variant)
tcr.SetVariant(tt.variant.Name())
assert.Equal(t, string(tt.variant), tcr.GetSessionInfo().Variant)
})
}
Expand Down
3 changes: 1 addition & 2 deletions src/params/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ package params

import (
"github.com/murex/tcr/runmode"
"github.com/murex/tcr/variant"
"time"
)

Expand All @@ -37,7 +36,7 @@ type Params struct {
Toolchain string
MobTurnDuration time.Duration
AutoPush bool
Variant variant.Variant
Variant string
CommitFailures bool
PollingPeriod time.Duration
Mode runmode.RunMode
Expand Down
5 changes: 2 additions & 3 deletions src/params/params_test_data_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ package params

import (
"github.com/murex/tcr/runmode"
"github.com/murex/tcr/variant"
"time"
)

Expand All @@ -40,7 +39,7 @@ func AParamSet(builders ...func(params *Params)) *Params {
Toolchain: "",
MobTurnDuration: 0,
AutoPush: false,
Variant: variant.Relaxed,
Variant: "relaxed",
PollingPeriod: 0,
Mode: runmode.OneShot{},
VCS: "git",
Expand Down Expand Up @@ -110,7 +109,7 @@ func WithAutoPush(value bool) func(params *Params) {
}

// WithVariant sets the provided value as the variant to be used
func WithVariant(variant variant.Variant) func(params *Params) {
func WithVariant(variant string) func(params *Params) {
return func(params *Params) {
params.Variant = variant
}
Expand Down
39 changes: 34 additions & 5 deletions src/variant/variant.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,45 @@ SOFTWARE.

package variant

// Variant represents the possible values for the TCR Variant
// https://medium.com/@tdeniffel/tcr-variants-test-commit-revert-bf6bd84b17d3
type Variant string
import "fmt"

func (v Variant) Name() string {
return string(v)
// UnsupportedVariantError is returned when the provided Variant name is not supported.
type UnsupportedVariantError struct {
variantName string
}

// Error returns the error description
func (e *UnsupportedVariantError) Error() string {
return fmt.Sprintf("variant not supported: \"%s\"", e.variantName)
}

// Variant represents the possible values for the TCR Variant.
// These values are inspired by the following blog-post:
// https://medium.com/@tdeniffel/tcr-variants-test-commit-revert-bf6bd84b17d3
type Variant string

// Recognized variant values
const (
Relaxed Variant = "relaxed"
BTCR Variant = "btcr"
Introspective Variant = "introspective"
)

var recognized = []Variant{Relaxed, BTCR, Introspective}

// Select returns a variant instance for the provided name.
// It returns an UnsupportedVariantError if the name is not recognized as a
// valid variant name.
func Select(name string) (*Variant, error) {
for _, variant := range recognized {
if name == variant.Name() {
return &variant, nil
}
}
return nil, &UnsupportedVariantError{name}
}

// Name returns the variant name
func (v Variant) Name() string {
return string(v)
}
27 changes: 27 additions & 0 deletions src/variant/variant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,30 @@ func Test_get_variant_name(t *testing.T) {
})
}
}

func Test_select_variant(t *testing.T) {
relaxed, btcr, introspective := Relaxed, BTCR, Introspective
tests := []struct {
name string
expectedVariant *Variant
expectedError error
}{
{"relaxed", &relaxed, nil},
{"btcr", &btcr, nil},
{"introspective", &introspective, nil},
{"unknown", nil, &UnsupportedVariantError{"unknown"}},
{"", nil, &UnsupportedVariantError{""}},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
variant, err := Select(test.name)
assert.Equal(t, test.expectedVariant, variant)
assert.Equal(t, test.expectedError, err)
})
}
}

func Test_unsupported_variant_message_format(t *testing.T) {
err := UnsupportedVariantError{"some-variant"}
assert.Equal(t, "variant not supported: \"some-variant\"", err.Error())
}

0 comments on commit e5eb2ba

Please sign in to comment.