chore: Introduce convenience methods for intstr. (#3702)

.github/workflows/ci-build.yaml

@@ -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 test-report
       - name: Upload test results
         if: failure()
         uses: actions/upload-artifact@v1

Makefile

@@ -320,6 +320,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: test-report
+test-report: test-results/junit.xml
+	go run ./hack test-report
+
 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

hack/main.go

@@ -18,6 +18,8 @@ func main() {
 		secondarySwaggerGen()
 	case "parseexamples":
 		parseExamples()
+	case "test-report":
+		testReport()
 	default:
 		panic(os.Args[1])
 	}

hack/test_report.go

@@ -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 testReport() {
+	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)
+			}
+		}
+	}
+}

util/intstr/parse.go

@@ -0,0 +1,9 @@
+package intstr
+
+import "k8s.io/apimachinery/pkg/util/intstr"
+
+// convenience func to get a pointer
+func ParsePtr(val string) *intstr.IntOrString {
+	x := intstr.Parse(val)
+	return &x
+}

util/printer/workflow-printer_test.go

@@ -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.ParsePtr("my-value")},
 				}},
 				Priority: pointer.Int32Ptr(2),
 				Templates: []wfv1.Template{

workflow/controller/cache_test.go

@@ -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          \\____\\______/   "
@@ -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.ParsePtr(MockParamValue),
 	}
 	cancel, controller := newController()
 	defer cancel()

workflow/controller/operator.go

@@ -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"
@@ -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"
@@ -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.ParsePtr(val)
 			param.ValueFrom = nil
 			outputs.Parameters = append(outputs.Parameters, param)
 		}

workflow/controller/operator_test.go

@@ -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"
@@ -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.ParsePtr("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)
@@ -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.ParsePtr("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"])
 
 }
@@ -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.ParsePtr("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.ParsePtr("101"),
+		Count: intstrutil.ParsePtr("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.ParsePtr("50"),
+		End:   intstrutil.ParsePtr("60"),
 	}
 	items, err = expandSequence(&seq)
 	assert.NoError(t, err)
@@ -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.ParsePtr("60"),
+		End:   intstrutil.ParsePtr("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.ParsePtr("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.ParsePtr("8"),
+		End:   intstrutil.ParsePtr("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.ParsePtr("10"),
+		Start:  intstrutil.ParsePtr("1"),
 	}
 	items, err = expandSequence(&seq)
 	assert.NoError(t, err)
@@ -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.ParsePtr(sampleOutput)},
 		},
 	}
 

workflow/controller/operator_workflow_template_ref_test.go

@@ -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) {
@@ -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.ParsePtr("test"),
 			},
 		}
 		wf.Spec.Arguments.Parameters = util.MergeParameters(wf.Spec.Arguments.Parameters, args)
@@ -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.ParsePtr("test"),
 			},
 		}
 		wftmpl.Spec.Arguments.Parameters = util.MergeParameters(wf.Spec.Arguments.Parameters, args)

workflow/controller/workflowpod_test.go

@@ -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"
 )
 
@@ -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.ParsePtr("test-name"),
 		},
 	}