wip: migrate e2e tests to godog framework (aka BDD/Cucumber) #2365

pedjak · 2025-11-28T15:11:56Z

Description

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

openshift-ci · 2025-11-28T15:12:02Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign perdasilva for approval. 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:

OWNERS

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

netlify · 2025-11-28T15:12:02Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`0482745`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/6929c58346652200086f6884
😎 Deploy Preview	https://deploy-preview-2365--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot

Pull request overview

This PR migrates the e2e test suite from traditional Go testing framework to Godog (BDD/Cucumber framework), enabling behavior-driven development with Gherkin feature files. The migration maintains test coverage while reorganizing tests into feature files with step definitions.

Key Changes

Replaced traditional Go test functions with Godog scenarios and step definitions
Added Gherkin .feature files describing test behavior in a more readable format
Introduced new test infrastructure (steps.go, hooks.go) to support BDD testing

Reviewed changes

Copilot reviewed 19 out of 21 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
test/e2e/features_test.go	New test entry point initializing Godog suite with scenario and suite initializers
test/e2e/features/steps/steps.go	Implements step definitions mapping Gherkin steps to Go functions
test/e2e/features/steps/hooks.go	Provides scenario lifecycle hooks and feature gate detection
test/e2e/features/*.feature	Gherkin feature files defining test scenarios (install, update, recover, metrics)
test/e2e/features/steps/testdata/*.yaml	YAML templates for test resources (catalogs, RBAC)
test/e2e/*_test.go	Removed traditional test files migrated to feature files
test/e2e/network_policy_test.go	Added client initialization and helper function
go.mod, go.sum	Added Cucumber/Godog dependencies

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-28T15:22:01Z

test/e2e/features/steps/steps.go

+var kubeconfigPath string
+
+func init() {
+	kubeconfigPath = os.Getenv("HOME") + "/.kube/config"


Using HOME environment variable for kubeconfig path is fragile. Consider using Go's standard approach with filepath.Join(os.UserHomeDir(), \".kube\", \"config\") or respect the KUBECONFIG environment variable convention for Kubernetes tooling.

Suggested change

kubeconfigPath = os.Getenv("HOME") + "/.kube/config"

kubeconfigPath = os.Getenv("KUBECONFIG")

if kubeconfigPath == "" {

home, err := os.UserHomeDir()

if err != nil {

panic(fmt.Sprintf("cannot determine user home directory: %v", err))

}

kubeconfigPath = filepath.Join(home, ".kube", "config")

}

Copilot · 2025-11-28T15:22:02Z

test/e2e/features/steps/steps.go

+		},
+	}
+	pb, err := json.Marshal(patch)
+	if err != nil {


Empty error handler. The error from json.Marshal is captured but not returned or handled, potentially hiding failures in catalog updates.

Suggested change

if err != nil {

if err != nil {

return fmt.Errorf("failed to marshal patch: %v", err)

Copilot · 2025-11-28T15:22:02Z

test/e2e/features/steps/hooks.go

+		p.Process.Kill()
+		p.Process.Wait()


Potential nil pointer dereference. If p.Process is nil (e.g., if Start() failed), this will panic. Add a nil check before calling Kill() and Wait().

Suggested change

p.Process.Kill()

p.Process.Wait()

if p.Process != nil {

p.Process.Kill()

p.Process.Wait()

}

Copilot · 2025-11-28T15:22:02Z

test/e2e/features/steps/steps.go

+	tr := &http.Transport{
+		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},


TLS certificate verification is disabled with InsecureSkipVerify: true. This creates a security vulnerability allowing man-in-the-middle attacks. For test code, consider using proper certificate handling or at minimum document why this is acceptable for the test environment.

Suggested change

tr := &http.Transport{

TLSClientConfig: &tls.Config{InsecureSkipVerify: true},

// Load CA certificate for test environment

caCertPath := filepath.Join("features", "steps", "testdata", "ca.crt")

caCert, err := os.ReadFile(caCertPath)

if err != nil {

return nil, fmt.Errorf("failed to read CA certificate: %v", err)

}

caCertPool := x509.NewCertPool()

if !caCertPool.AppendCertsFromPEM(caCert) {

return nil, fmt.Errorf("failed to append CA certificate")

}

tr := &http.Transport{

TLSClientConfig: &tls.Config{

RootCAs: caCertPool,

},

Copilot · 2025-11-28T15:22:02Z

test/e2e/features/steps/testdata/rbac-template.yaml

+  - apiGroups: [olm.operatorframework.io]
+    resources: [clusterextensions, clusterextensions/finalizers]
+    resourceNames: ["{clusterextension_name}"]
+    #verbs: [update, create, list, watch, get, delete, patch]


Commented-out verbs suggest incomplete implementation or unclear intent. Either remove the commented line if not needed, or add a comment explaining why only 'update' is required while other verbs are commented out.

Suggested change

#verbs: [update, create, list, watch, get, delete, patch]

Copilot · 2025-11-28T15:22:03Z

test/e2e/features_test.go

+
+		path := os.Getenv("E2E_SUMMARY_OUTPUT")
+		if path == "" {
+			fmt.Printf("Note: E2E_SUMMARY_OUTPUT is unset; skipping summary generation")


Missing newline character in printf statement. Should be fmt.Println() or add \\n to ensure proper output formatting.

Suggested change

fmt.Printf("Note: E2E_SUMMARY_OUTPUT is unset; skipping summary generation")

fmt.Println("Note: E2E_SUMMARY_OUTPUT is unset; skipping summary generation")

codecov · 2025-11-28T16:08:10Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.32%. Comparing base (4355cde) to head (0482745).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2365      +/-   ##
==========================================
- Coverage   74.45%   74.32%   -0.13%     
==========================================
  Files          93       93              
  Lines        7300     7300              
==========================================
- Hits         5435     5426       -9     
- Misses       1433     1441       +8     
- Partials      432      433       +1

Flag	Coverage Δ
e2e	`43.66% <ø> (-0.86%)`	⬇️
experimental-e2e	`47.87% <ø> (-0.92%)`	⬇️
unit	`58.47% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI review requested due to automatic review settings November 28, 2025 15:11

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 Nov 28, 2025

Copilot started reviewing on behalf of pedjak November 28, 2025 15:12 View session

Copilot finished reviewing on behalf of pedjak November 28, 2025 15:13

Copilot AI reviewed Nov 28, 2025

View reviewed changes

wip: migrate e2e tests to godog framework (aka BDD/Cucumber)

0482745

pedjak force-pushed the e2e-spec-by-example-godog branch from 730ba01 to 0482745 Compare November 28, 2025 15:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

wip: migrate e2e tests to godog framework (aka BDD/Cucumber) #2365

wip: migrate e2e tests to godog framework (aka BDD/Cucumber) #2365

Uh oh!

pedjak commented Nov 28, 2025

Uh oh!

openshift-ci bot commented Nov 28, 2025

Uh oh!

netlify bot commented Nov 28, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

Copilot AI Nov 28, 2025

Uh oh!

codecov bot commented Nov 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

-	kubeconfigPath = os.Getenv("HOME") + "/.kube/config"
+	kubeconfigPath = os.Getenv("KUBECONFIG")
+	if kubeconfigPath == "" {
+		home, err := os.UserHomeDir()
+		if err != nil {
+			panic(fmt.Sprintf("cannot determine user home directory: %v", err))
+		}
+		kubeconfigPath = filepath.Join(home, ".kube", "config")
+	}

	if err != nil {
	if err != nil {
	return fmt.Errorf("failed to marshal patch: %v", err)

		tr := &http.Transport{
		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},

-	tr := &http.Transport{
-		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+	// Load CA certificate for test environment
+	caCertPath := filepath.Join("features", "steps", "testdata", "ca.crt")
+	caCert, err := os.ReadFile(caCertPath)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read CA certificate: %v", err)
+	}
+	caCertPool := x509.NewCertPool()
+	if !caCertPool.AppendCertsFromPEM(caCert) {
+		return nil, fmt.Errorf("failed to append CA certificate")
+	}
+	tr := &http.Transport{
+		TLSClientConfig: &tls.Config{
+			RootCAs: caCertPool,
+		},

	fmt.Printf("Note: E2E_SUMMARY_OUTPUT is unset; skipping summary generation")
	fmt.Println("Note: E2E_SUMMARY_OUTPUT is unset; skipping summary generation")

wip: migrate e2e tests to godog framework (aka BDD/Cucumber) #2365

Are you sure you want to change the base?

wip: migrate e2e tests to godog framework (aka BDD/Cucumber) #2365

Uh oh!

Conversation

pedjak commented Nov 28, 2025

Description

Reviewer Checklist

Uh oh!

openshift-ci bot commented Nov 28, 2025

Uh oh!

netlify bot commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 28, 2025

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

netlify bot commented Nov 28, 2025 •

edited

Loading