Skip to content

WIP: Added integration.go file #1331

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

WIP: Added integration.go file #1331

wants to merge 1 commit into from

Conversation

mmakwana30
Copy link

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 15, 2025
@openshift-ci openshift-ci bot requested review from eggfoobar and qJkee September 15, 2025 09:45
@mmakwana30
Copy link
Author

/assign @jeff-roche @kasturinarra

kasturinarra
kasturinarra reviewed Sep 15, 2025
View reviewed changes
test/integration/integration.go
@@ -0,0 +1,53 @@
package main
Copy link

Choose a reason for hiding this comment

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

I would suggest creating a new file called main.go inside cmd/integration-tests/main.go with package main.

Copy link
Contributor

Choose a reason for hiding this comment

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

The file name doesn't particularly matter since we can feed it directly to the go build command. I believe best practice is to only have 1 main.go in a repo which we already have in the cmd folder so I think leaving it as integration here is fine

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Sep 15, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mmakwana30
Once this PR has been reviewed and has the lgtm label, please ask for approval from jeff-roche. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kasturinarra
kasturinarra reviewed Sep 15, 2025
View reviewed changes
test/integration/integration.go Outdated
"fmt"
"os"

et "github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests"
Copy link

Choose a reason for hiding this comment

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

I see we are not using the et anywhere, this can be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would just comment it out for now so that we don't end up with compile errors. We will want it down the road though

kasturinarra
kasturinarra reviewed Sep 15, 2025
View reviewed changes
test/integration/integration.go Outdated

// all test cases
ext.AddSuite(e.Suite{
Name: "openshift/lvm-operator/test",
Copy link

@kasturinarra kasturinarra Sep 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Better naming would be openshift/lvm-operator/integration or openshift/lvm-operator/integration-tests ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like openshift/lvm-operator/integration

kasturinarra
kasturinarra reviewed Sep 15, 2025
View reviewed changes
test/integration/integration.go
registry := e.NewRegistry()

// You can declare multiple extensions, but most people will probably only need to create one.
ext := e.NewExtension("openshift", "payload", "lvm-operator")
Copy link

Choose a reason for hiding this comment

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

I am not very sure about the payload here, but i will let jeff comment more on that as i am not very sure what should we be selecting.

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't find any validation in the source so we will go with payload for now and see if it works

jeff-roche
jeff-roche requested changes Sep 15, 2025
View reviewed changes
Copy link
Contributor

@jeff-roche jeff-roche left a comment

Choose a reason for hiding this comment

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

Let's also add a basic dockerfile with the go build and makefile extension as part of this PR

@jeff-roche jeff-roche changed the title Added integration.go file WIP: Added integration.go file Sep 16, 2025
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 16, 2025
@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 16, 2025
jeff-roche
jeff-roche reviewed Sep 16, 2025
View reviewed changes
jeff-roche
jeff-roche reviewed Sep 16, 2025
View reviewed changes
jeff-roche
jeff-roche reviewed Sep 16, 2025
View reviewed changes
@mmakwana30 mmakwana30 force-pushed the integration branch 3 times, most recently from 6917cb0 to 190a869 Compare September 16, 2025 17:01
@mmakwana30
Added integration directory with integration.go file
0884cd8 
Added integration directory with integration.go file
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Sep 16, 2025

@mmakwana30: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/precommit-check 0884cd8 link true /test precommit-check
ci/prow/e2e-aws-hypershift 0884cd8 link true /test e2e-aws-hypershift
ci/prow/unit-test 0884cd8 link true /test unit-test
ci/prow/snyk-deps 0884cd8 link false /test snyk-deps
ci/prow/e2e-aws-single-node 0884cd8 link true /test e2e-aws-single-node
ci/prow/e2e-aws 0884cd8 link true /test e2e-aws

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeff-roche jeff-roche jeff-roche requested changes

@eggfoobar eggfoobar Awaiting requested review from eggfoobar

@qJkee qJkee Awaiting requested review from qJkee

+1 more reviewer

@kasturinarra kasturinarra kasturinarra left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@kasturinarra kasturinarra

@jeff-roche jeff-roche

Labels
do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mmakwana30 @kasturinarra @jeff-roche