Skip to content

Commit

Permalink
review feedback
Browse files Browse the repository at this point in the history
* Address review feedback
* Some of the tests results are modified due some changes is sorting in
  the various algorithms.

Signed-off-by: Ryan Cragun <[email protected]>
  • Loading branch information
ryancragun committed Aug 28, 2023
1 parent 804bf90 commit 2c64088
Show file tree
Hide file tree
Showing 17 changed files with 288 additions and 214 deletions.
2 changes: 1 addition & 1 deletion .go-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.20
1.21
4 changes: 1 addition & 3 deletions acceptance/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"time"

"github.com/aws/aws-sdk-go-v2/config"
"github.com/davecgh/go-spew/spew"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/encoding/protojson"

Expand Down Expand Up @@ -240,8 +239,7 @@ func requireEqualOperationResponses(t *testing.T, expected *pb.OperationResponse

got := &pb.OperationResponses{}
require.NoErrorf(t, protojson.Unmarshal(out, got), string(out))
require.Lenf(t, expected.GetResponses(), len(got.GetResponses()),
spew.Sdump(expected.GetResponses(), got.GetResponses()))
require.Len(t, expected.GetResponses(), len(got.GetResponses()))
expectedResponses := expected.GetResponses()
gotResponses := got.GetResponses()
sortResponses(expectedResponses)
Expand Down
24 changes: 12 additions & 12 deletions acceptance/scenario_sample_observe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ func TestAcc_Cmd_Scenario_Sample_Observe(t *testing.T) {
Scenario: &pb.Ref_Scenario{
Id: &pb.Scenario_ID{
Name: "smoke",
Uid: "54d1ffff5afd49c7e03c636cc8d4b9c936e3c05c91ba6e3e0464b4e2bc7b84d9",
Filter: "smoke arch:amd64 distro:sles",
Uid: "ebc8083d61b560ea1678efb642cfdf54034a7374e18d27e757e71bb5dc28c62e",
Filter: "smoke arch:arm64 distro:rhel",
Variants: &pb.Matrix_Vector{
Elements: []*pb.Matrix_Element{
{Key: "arch", Value: "amd64"},
{Key: "distro", Value: "sles"},
{Key: "arch", Value: "arm64"},
{Key: "distro", Value: "rhel"},
},
},
},
Expand All @@ -83,12 +83,12 @@ func TestAcc_Cmd_Scenario_Sample_Observe(t *testing.T) {
Scenario: &pb.Ref_Scenario{
Id: &pb.Scenario_ID{
Name: "smoke",
Uid: "bdc1d6d8a1d8332d0ee5ea09e1cb6ad06d47953f8e225de8dc1f0ec3be5eb6a0",
Filter: "smoke arch:s390x distro:amz",
Uid: "21610357b0083126896a0072a429e677fd6381fa98e94b15ac2101f697100b42",
Filter: "smoke arch:s390x distro:ubuntu",
Variants: &pb.Matrix_Vector{
Elements: []*pb.Matrix_Element{
{Key: "arch", Value: "s390x"},
{Key: "distro", Value: "amz"},
{Key: "distro", Value: "ubuntu"},
},
},
},
Expand All @@ -115,19 +115,19 @@ func TestAcc_Cmd_Scenario_Sample_Observe(t *testing.T) {
Scenario: &pb.Ref_Scenario{
Id: &pb.Scenario_ID{
Name: "upgrade",
Uid: "bbc17b16f2ef5a6d7011692e29b1783083ec397626a73615c2c2f7b19642c1f0",
Filter: "upgrade arch:s390x distro:rhel",
Uid: "f662b9deac220ad5bc22f2618098925b47d06fc342d8c0d0e6240bb67547b9e8",
Filter: "upgrade arch:arm64 distro:ubuntu",
Variants: &pb.Matrix_Vector{
Elements: []*pb.Matrix_Element{
{Key: "arch", Value: "s390x"},
{Key: "distro", Value: "rhel"},
{Key: "arch", Value: "arm64"},
{Key: "distro", Value: "ubuntu"},
},
},
},
},
Attributes: &structpb.Struct{
Fields: map[string]*structpb.Value{
"aws-region": structpb.NewStringValue("us-east-1"),
"aws-region": structpb.NewStringValue("us-west-1"),
"continue-on-error": structpb.NewBoolValue(false),
},
},
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ go 1.21.0
require (
github.com/Masterminds/semver/v3 v3.2.1
github.com/aws/aws-sdk-go-v2/config v1.18.28
github.com/davecgh/go-spew v1.1.1
github.com/google/uuid v1.3.0
github.com/hashicorp/go-hclog v1.5.0
github.com/hashicorp/go-multierror v1.1.1
Expand Down Expand Up @@ -45,6 +44,7 @@ require (
github.com/aws/aws-sdk-go-v2/service/sts v1.19.3 // indirect
github.com/aws/smithy-go v1.13.5 // indirect
github.com/bgentry/speakeasy v0.1.0 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/fatih/color v1.15.0 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/google/go-cmp v0.5.9 // indirect
Expand Down
2 changes: 1 addition & 1 deletion internal/command/enos/cmd/scenario_sample_observe.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func NewScenarioSampleObserveCmd() *cobra.Command {
sampleObserveCmd := &cobra.Command{
Use: "observe [sample_name] [args]",
Short: "Take an observation of the scenario sample",
Long: "Take an observation of the scenario sample and return the scenario elements. The sample frame is the aggregate of each subset's frame. Each subset frame is created by expanding its scenario filter and/or matrix into a cartersian product of possible scenario and variant combinations. By default all subsets in the sample are included in the frame but the inclusion or exclusion of subsets in the frame can be done using the include and exclude flags. The number of sample elements to include in the observation can be controlled using the min, max, and pct flags. If a max is set to a negative number it will set no default upper bound. If a percentage is set it will create an upper bound of the percentage of the total frame. If both a max and percentage are set the lowest will be used as the upper bound. By default a minimum is set to 1. If the minimum is not possible an error will be returned. The seed can be used to control the random number source to create deterministic observations. If the seed number is negative a seed will be generated for you.",
Long: `Take an observation of the scenario sample. This returns a list of all the possible scenarios/variant included in the subsets for the sample (also known as the sample frame). The observation must be limited to a particular sample by passing the sample name as an argument. The sample frame can be limited by using --include or --exclude flags. The number of randomly selected scenarios to observe can be limited using the min (minimum number of scenarios elements to return), max (maximum number of scenarios elements to return), and pct (limit then the overall possible scenarios as a percentage of total scenarios included in the frame) flags. If the max is set to a negative number, it will set no default upper bound. If a pct is set, it will create an upper bound of the percentage of the total sample frame. If both a max and a pct are set, whichever is lower will be used as the upper bound. By default, the min is set to 1 unless otherwise specified. If a min is set that is higher than the actual number of scenarios in the frame, an error will be returned. If replicable sample is desired (you can execute the observe command and get the same results) an entropy seed can be used to control the random number source. If no seed is given a random one will be chosen for you.`,
RunE: runSampleShowCmd,
Args: cobra.ExactArgs(1), // The sample name
}
Expand Down
6 changes: 4 additions & 2 deletions internal/flightplan/matrix.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,8 +555,10 @@ func (m *Matrix) Filter(filter *ScenarioFilter) *Matrix {
return nm
}

// IntersectionContainsUnordered returns an intersection of both matrices where each vector
// contains the unordered elements of the other vector.
// IntersectionContainsUnordered takes another matrix and returns a new matrix whose vectors are
// composed of the result of a intersection of both matrices vector elements that are contained and
// unordered. It's important to not that contains does not mean equal, so a vector [1,2,3] contains
// [3,1] but a vector of [3,1] does not contain [1,2,3] because it doesn't have all of the elements.
func (m *Matrix) IntersectionContainsUnordered(other *Matrix) *Matrix {
if m == nil || other == nil {
return nil
Expand Down
24 changes: 12 additions & 12 deletions internal/flightplan/sample_frame_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ module "foo" {
scenario "foo" {
matrix {
length = ["fl1", "fl2", "fl3"]
width = ["fw1", "fw2", "fw3"]
width = ["fw1", "fw2", "fw3"]
}
step "foo" {
Expand All @@ -73,7 +73,7 @@ scenario "foo" {
scenario "bar" {
matrix {
length = ["bl1", "bl2", "bl3"]
width = ["bw1", "bw2", "bw3"]
width = ["bw1", "bw2", "bw3"]
}
step "foo" {
Expand All @@ -94,35 +94,35 @@ sample "all" {
}
subset "merge" {
scenario_name = "foo"
scenario_name = "foo"
attributes = {
test-group = "merge"
test-group = "merge"
}
matrix {
length = ["fl1", "fl2"]
width = ["fw1", "fw2"]
length = ["fl1", "fl2"]
width = ["fw1", "fw2"]
}
}
subset "override" {
scenario_name = "bar"
scenario_name = "bar"
attributes = {
test-group = "override"
test-group = "override"
continue-on-error = true // Overridden attributes
aws-region = ["eu-west-1", "us-east-1"]
aws-region = ["eu-west-1", "us-east-1"]
}
matrix {
length = ["bl1", "bl2"]
width = ["bw1", "bw2"]
length = ["bl1", "bl2"]
width = ["bw1", "bw2"]
}
}
subset "nomatrix" {
scenario_name = "baz"
scenario_name = "baz"
}
}`, modulePath)
for subsetName, test := range map[string]struct {
Expand Down
Loading

0 comments on commit 2c64088

Please sign in to comment.