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

SRVKP-6722 new tests for versioned ecosystem tasks #543

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jayesh-garg
Copy link
Collaborator

No description provided.

@jayesh-garg jayesh-garg force-pushed the SRVKP-6722 branch 5 times, most recently from 6d8ab97 to 9396126 Compare February 4, 2025 12:11
@jayesh-garg jayesh-garg self-assigned this Feb 4, 2025
@@ -63,4 +63,15 @@ Importance: Critical
Pos/Neg: Negative
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this have 05 in ID? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope... we removed IDs 1 to 5 and we shouldn't reuse the IDs. This one should be 09, following ones 10 and 11


## Verify versioned ecosystem tasks: PIPELINES-15-TC09
Tags: e2e, integration, admin, addon, sanity
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't need to run as admin, remove sanity

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

specs/operator/addon.spec Outdated Show resolved Hide resolved
steps/cli/oc.go Outdated
@@ -165,6 +166,16 @@ var _ = gauge.Step("Update addon config with resolverStepActions as <resolverSte
}
})

var _ = gauge.Step("Verify test versioned ecosystem task", func() {
taskList := cmd.MustSucceed("oc", "get", "task", "-n", "openshift-pipelines").Stdout()
requriedTasks := []string{"git-cli", "git-clone", "s2i-go", "s2i-python"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. typo
  2. extend this list to all tasks

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

steps/cli/oc.go Show resolved Hide resolved
steps/cli/oc.go Outdated
taskList := cmd.MustSucceed("oc", "get", "task", "-n", "openshift-pipelines").Stdout()
requriedTasks := []string{"git-cli", "git-clone", "s2i-go", "s2i-python"}
for _, task := range requriedTasks {
if !strings.Contains(taskList, task) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are supposed to check versioned tasks but you don't check those.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

steps/cli/oc.go Outdated
requriedTasks := []string{"git-cli", "git-clone", "s2i-go", "s2i-python"}
for _, task := range requriedTasks {
if !strings.Contains(taskList, task) {
testsuit.T.Errorf("Failed to test versioned task, No task found!!")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provide context in error message ... which task was not found?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -63,4 +63,15 @@ Importance: Critical
Pos/Neg: Negative
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope... we removed IDs 1 to 5 and we shouldn't reuse the IDs. This one should be 09, following ones 10 and 11


Steps:
* Verify versioned stepaction tasks
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Verify versioned stepaction tasks
* Verify versioned ecosystem step actions

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

steps/cli/oc.go Outdated
var _ = gauge.Step("Verify versioned ecosystem tasks", func() {
taskList := cmd.MustSucceed("oc", "get", "task", "-n", "openshift-pipelines").Stdout()
requiredTasks := []string{"buildah", "git-cli", "git-clone", "kn", "kn-apply", "maven", "openshift-client", "s2i-dotnet", "s2i-go", "s2i-java", "s2i-nodejs", "s2i-perl", "s2i-php", "s2i-python", "s2i-ruby", "skopeo-copy", "tkn"}
requiredVersions := []string{"1-16-0", "1-17-0"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot hard-code versions here, that would increase maintenance burden. Just read the expected version from env variable OSP_VERSION as defined env/default/default.properties. You need to transform the string but that is not a big deal. It's enough to verify current version, you don't need to check all versions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

steps/cli/oc.go Outdated
for _, version := range requiredVersions {
taskWithVersion := task + "-" + version
if !strings.Contains(taskList, taskWithVersion) {
testsuit.T.Errorf("Failed to test versioned task, No task found: %s", taskWithVersion)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
testsuit.T.Errorf("Failed to test versioned task, No task found: %s", taskWithVersion)
testsuit.T.Errorf("Task %s not found in namespace openshift-pipelines", taskWithVersion)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

steps/cli/oc.go Outdated
}
})

var _ = gauge.Step("Verify versioned stepaction tasks", func() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants