Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Introduce convenience methods for intstr. #3702

Merged
merged 15 commits into from
Aug 9, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/ci-build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ jobs:
run: |
trap 'make test-results/junit.xml' EXIT
make ${{ matrix.test }}
- name: Print test error annotations
if: failure()
run: make printtestresultannotations
- name: Upload test results
if: failure()
uses: actions/upload-artifact@v1
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,10 @@ $(GOPATH)/bin/go-junit-report:
test-results/junit.xml: $(GOPATH)/bin/go-junit-report test-results/test.out
cat test-results/test.out | go-junit-report > test-results/junit.xml

.PHONY: printtestresultannotations
printtestresultannotations:
alexec marked this conversation as resolved.
Show resolved Hide resolved
go run ./hack printtestresultannotations

dist/$(PROFILE).yaml: $(MANIFESTS) $(E2E_MANIFESTS)
mkdir -p dist
kustomize build --load_restrictor=none test/e2e/manifests/$(PROFILE) | sed 's/:latest/:$(VERSION)/' | sed 's/pns/$(E2E_EXECUTOR)/' > dist/$(PROFILE).yaml
Expand Down
2 changes: 2 additions & 0 deletions hack/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ func main() {
secondarySwaggerGen()
case "parseexamples":
parseExamples()
case "printtestresultannotations":
printTestResultAnnotations()
default:
panic(os.Args[1])
}
Expand Down
51 changes: 51 additions & 0 deletions hack/print_test_error_annotations.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package main

import (
"encoding/xml"
"fmt"
"io/ioutil"
"strings"
)

type failure struct {
Text string `xml:",chardata"`
}

type testcase struct {
Failure failure `xml:"failure,omitempty"`
}

type testsuite struct {
Name string `xml:"name,attr"`
TestCases []testcase `xml:"testcase"`
}

type report struct {
XMLName xml.Name `xml:"testsuites"`
TestSuites []testsuite `xml:"testsuite"`
}

func printTestResultAnnotations() {
alexec marked this conversation as resolved.
Show resolved Hide resolved
data, err := ioutil.ReadFile("test-results/junit.xml")
if err != nil {
panic(err)
}
v := &report{}
err = xml.Unmarshal(data, v)
if err != nil {
panic(err)
}
for _, s := range v.TestSuites {
for _, c := range s.TestCases {
if c.Failure.Text != "" {
// https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message
// Replace ‘/n’ with ‘%0A’ for multiple strings output.
parts := strings.SplitN(c.Failure.Text, ":", 3)
file := strings.ReplaceAll(s.Name, "github.com/argoproj/argo/", "") + "/" + parts[0]
line := parts[1]
message := strings.ReplaceAll(strings.TrimSpace(parts[2]), "\n", "%0A")
_, _ = fmt.Printf("::error file=%s,line=%v,col=0::%s\n", file, line, message)
}
}
}
}
9 changes: 9 additions & 0 deletions util/intstr/parse.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package intstr

import "k8s.io/apimachinery/pkg/util/intstr"

// convenience func to get a pointer
func Parse(val string) *intstr.IntOrString {
alexec marked this conversation as resolved.
Show resolved Hide resolved
x := intstr.Parse(val)
return &x
}
5 changes: 2 additions & 3 deletions util/printer/workflow-printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,20 @@ import (
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/utils/pointer"

wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
intstrutil "github.com/argoproj/argo/util/intstr"
)

func TestPrintWorkflows(t *testing.T) {
now := time.Now()
intOrString := intstr.Parse("my-value")
workflows := wfv1.Workflows{
{
ObjectMeta: metav1.ObjectMeta{Name: "my-wf", Namespace: "my-ns", CreationTimestamp: metav1.Time{Time: now}},
Spec: wfv1.WorkflowSpec{
Arguments: wfv1.Arguments{Parameters: []wfv1.Parameter{
{Name: "my-param", Value: &intOrString},
{Name: "my-param", Value: intstrutil.Parse("my-value")},
}},
Priority: pointer.Int32Ptr(2),
Templates: []wfv1.Template{
Expand Down
12 changes: 4 additions & 8 deletions workflow/controller/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,13 @@ package controller
import (
"testing"

"github.com/argoproj/argo/workflow/controller/cache"

"k8s.io/apimachinery/pkg/util/intstr"

apiv1 "k8s.io/api/core/v1"

"github.com/stretchr/testify/assert"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
intstrutil "github.com/argoproj/argo/util/intstr"
"github.com/argoproj/argo/workflow/controller/cache"
)

var sampleOutput string = "\n__________ \n\u003c hi there \u003e\n ---------- \n \\\n \\\n \\ \n ## . \n ##\n## ## == \n ## ## ## ## === \n /\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"___/\n=== \n ~~~ {~~ ~~~~ ~~~ ~~~~ ~~ ~ / ===- ~~~ \n \\______ o __/ \n \\ \\ __/ \n \\____\\______/ "
Expand Down Expand Up @@ -70,10 +67,9 @@ func TestConfigMapCacheLoadMiss(t *testing.T) {

func TestConfigMapCacheSave(t *testing.T) {
var MockParamValue string = "Hello world"
val := intstr.Parse(MockParamValue)
var MockParam = wfv1.Parameter{
Name: "hello",
Value: &val,
Value: intstrutil.Parse(MockParamValue),
}
cancel, controller := newController()
defer cancel()
Expand Down
5 changes: 2 additions & 3 deletions workflow/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"sync"
"time"

"github.com/argoproj/argo/util/intstr"
controllercache "github.com/argoproj/argo/workflow/controller/cache"

"github.com/argoproj/pkg/humanize"
Expand All @@ -25,7 +26,6 @@ import (
apierr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"
Expand Down Expand Up @@ -2060,8 +2060,7 @@ func getTemplateOutputsFromScope(tmpl *wfv1.Template, scope *wfScope) (*wfv1.Out
return nil, err
}
}
intOrString := intstr.Parse(val)
param.Value = &intOrString
param.Value = intstr.Parse(val)
param.ValueFrom = nil
outputs.Parameters = append(outputs.Parameters, param)
}
Expand Down
53 changes: 21 additions & 32 deletions workflow/controller/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/intstr"
"sigs.k8s.io/yaml"

"github.com/argoproj/argo/config"
wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
"github.com/argoproj/argo/test"
intstrutil "github.com/argoproj/argo/util/intstr"
"github.com/argoproj/argo/workflow/common"
"github.com/argoproj/argo/workflow/controller/cache"
hydratorfake "github.com/argoproj/argo/workflow/hydrator/fake"
Expand Down Expand Up @@ -1871,10 +1871,10 @@ func TestWorkflowSpecParam(t *testing.T) {
func TestAddGlobalParamToScope(t *testing.T) {
woc := newWoc()
woc.globalParams = make(map[string]string)
testVal := intstr.Parse("test-value")
testVal := intstrutil.Parse("test-value")
param := wfv1.Parameter{
Name: "test-param",
Value: &testVal,
Value: testVal,
}
// Make sure if the param is not global, don't add to scope
woc.addParamToGlobalScope(param)
Expand All @@ -1885,24 +1885,24 @@ func TestAddGlobalParamToScope(t *testing.T) {
woc.addParamToGlobalScope(param)
assert.Equal(t, 1, len(woc.wf.Status.Outputs.Parameters))
assert.Equal(t, param.GlobalName, woc.wf.Status.Outputs.Parameters[0].Name)
assert.Equal(t, testVal, *woc.wf.Status.Outputs.Parameters[0].Value)
assert.Equal(t, testVal, woc.wf.Status.Outputs.Parameters[0].Value)
assert.Equal(t, testVal.String(), woc.globalParams["workflow.outputs.parameters.global-param"])

// Change the value and verify it is reflected in workflow outputs
newValue := intstr.Parse("new-value")
param.Value = &newValue
newValue := intstrutil.Parse("new-value")
param.Value = newValue
woc.addParamToGlobalScope(param)
assert.Equal(t, 1, len(woc.wf.Status.Outputs.Parameters))
assert.Equal(t, param.GlobalName, woc.wf.Status.Outputs.Parameters[0].Name)
assert.Equal(t, newValue, *woc.wf.Status.Outputs.Parameters[0].Value)
assert.Equal(t, newValue, woc.wf.Status.Outputs.Parameters[0].Value)
assert.Equal(t, newValue.String(), woc.globalParams["workflow.outputs.parameters.global-param"])

// Add a new global parameter
param.GlobalName = "global-param2"
woc.addParamToGlobalScope(param)
assert.Equal(t, 2, len(woc.wf.Status.Outputs.Parameters))
assert.Equal(t, param.GlobalName, woc.wf.Status.Outputs.Parameters[1].Name)
assert.Equal(t, newValue, *woc.wf.Status.Outputs.Parameters[1].Value)
assert.Equal(t, newValue, woc.wf.Status.Outputs.Parameters[1].Value)
assert.Equal(t, newValue.String(), woc.globalParams["workflow.outputs.parameters.global-param2"])

}
Expand Down Expand Up @@ -1976,32 +1976,28 @@ func TestExpandWithSequence(t *testing.T) {
var items []wfv1.Item
var err error

ten := intstr.Parse("10")
seq = wfv1.Sequence{
Count: &ten,
Count: intstrutil.Parse("10"),
}
items, err = expandSequence(&seq)
assert.NoError(t, err)
assert.Equal(t, 10, len(items))
assert.Equal(t, "0", items[0].GetStrVal())
assert.Equal(t, "9", items[9].GetStrVal())

oneOhOne := intstr.Parse("101")
seq = wfv1.Sequence{
Start: &oneOhOne,
Count: &ten,
Start: intstrutil.Parse("101"),
Count: intstrutil.Parse("10"),
}
items, err = expandSequence(&seq)
assert.NoError(t, err)
assert.Equal(t, 10, len(items))
assert.Equal(t, "101", items[0].GetStrVal())
assert.Equal(t, "110", items[9].GetStrVal())

fifty := intstr.Parse("50")
sixty := intstr.Parse("60")
seq = wfv1.Sequence{
Start: &fifty,
End: &sixty,
Start: intstrutil.Parse("50"),
End: intstrutil.Parse("60"),
}
items, err = expandSequence(&seq)
assert.NoError(t, err)
Expand All @@ -2010,38 +2006,35 @@ func TestExpandWithSequence(t *testing.T) {
assert.Equal(t, "60", items[10].GetStrVal())

seq = wfv1.Sequence{
Start: &sixty,
End: &fifty,
Start: intstrutil.Parse("60"),
End: intstrutil.Parse("50"),
}
items, err = expandSequence(&seq)
assert.NoError(t, err)
assert.Equal(t, 11, len(items))
assert.Equal(t, "60", items[0].GetStrVal())
assert.Equal(t, "50", items[10].GetStrVal())

zero := intstr.Parse("0")
seq = wfv1.Sequence{
Count: &zero,
Count: intstrutil.Parse("0"),
}
items, err = expandSequence(&seq)
assert.NoError(t, err)
assert.Equal(t, 0, len(items))

eight := intstr.Parse("8")
seq = wfv1.Sequence{
Start: &eight,
End: &eight,
Start: intstrutil.Parse("8"),
End: intstrutil.Parse("8"),
}
items, err = expandSequence(&seq)
assert.NoError(t, err)
assert.Equal(t, 1, len(items))
assert.Equal(t, "8", items[0].GetStrVal())

one := intstr.Parse("1")
seq = wfv1.Sequence{
Format: "testuser%02X",
Count: &ten,
Start: &one,
Count: intstrutil.Parse("10"),
Start: intstrutil.Parse("1"),
}
items, err = expandSequence(&seq)
assert.NoError(t, err)
Expand Down Expand Up @@ -4067,13 +4060,9 @@ func TestConfigMapCacheSaveOperate(t *testing.T) {
defer cancel()

woc := newWorkflowOperationCtx(wf, controller)
outputVal := intstr.Parse(sampleOutput)
sampleOutputs := wfv1.Outputs{
Parameters: []wfv1.Parameter{
wfv1.Parameter{
Name: "hello",
Value: &outputVal,
},
{Name: "hello", Value: intstrutil.Parse(sampleOutput)},
},
}

Expand Down
8 changes: 3 additions & 5 deletions workflow/controller/operator_workflow_template_ref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/util/intstr"

wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
"github.com/argoproj/argo/util"
intstrutil "github.com/argoproj/argo/util/intstr"
)

func TestWorkflowTemplateRef(t *testing.T) {
Expand All @@ -29,11 +29,10 @@ func TestWorkflowTemplateRefWithArgs(t *testing.T) {
wftmpl := unmarshalWFTmpl(wfTmpl)

t.Run("CheckArgumentPassing", func(t *testing.T) {
value := intstr.Parse("test")
args := []wfv1.Parameter{
{
Name: "param1",
Value: &value,
Value: intstrutil.Parse("test"),
},
}
wf.Spec.Arguments.Parameters = util.MergeParameters(wf.Spec.Arguments.Parameters, args)
Expand All @@ -50,11 +49,10 @@ func TestWorkflowTemplateRefWithWorkflowTemplateArgs(t *testing.T) {
wftmpl := unmarshalWFTmpl(wfTmpl)

t.Run("CheckArgumentFromWFT", func(t *testing.T) {
value := intstr.Parse("test")
args := []wfv1.Parameter{
{
Name: "param1",
Value: &value,
Value: intstrutil.Parse("test"),
},
}
wftmpl.Spec.Arguments.Parameters = util.MergeParameters(wf.Spec.Arguments.Parameters, args)
Expand Down
5 changes: 2 additions & 3 deletions workflow/controller/workflowpod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import (
"github.com/stretchr/testify/assert"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/utils/pointer"
"sigs.k8s.io/yaml"

"github.com/argoproj/argo/config"
wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
"github.com/argoproj/argo/test/util"
intstrutil "github.com/argoproj/argo/util/intstr"
"github.com/argoproj/argo/workflow/common"
)

Expand Down Expand Up @@ -658,11 +658,10 @@ func TestVolumesPodSubstitution(t *testing.T) {
MountPath: "/test",
},
}
tmpStr := intstr.Parse("test-name")
inputParameters := []wfv1.Parameter{
{
Name: "volume-name",
Value: &tmpStr,
Value: intstrutil.Parse("test-name"),
},
}

Expand Down
Loading