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.

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

⚠️ Update ginkgo@v2.1.1 and gomega@v1.18.0 #1792

Closed
wants to merge 13 commits into from
Closed

⚠️ Update ginkgo@v2.1.1 and gomega@v1.18.0 #1792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
92b5ae5
Upgrade to ginkgo v2.1.1
acumino Feb 4, 2022
ed8df81
Adapt ginkgo import paths
acumino Feb 4, 2022
6bf50a9
Drop obsolete table extension imports
acumino Feb 4, 2022
fed90d2
tidy
acumino Feb 4, 2022
d491a45
Remove {Before,After} Suite specific timeout
acumino Feb 4, 2022
486e94d
Remove It specific timeout
acumino Feb 4, 2022
c784cdf
Adapt custom reporter to changed reporting infrastructure
acumino Feb 5, 2022
3ba9150
Update dependency
acumino Feb 16, 2022
f225518
Address PR Review
acumino Feb 16, 2022
46ea91f
Adpat custom reporter chnages
acumino Mar 3, 2022
50edea9
Minor edit
acumino Mar 3, 2022
a266794
Rebase
acumino Mar 14, 2022
1a677d0
Rebase
acumino Jun 3, 2022
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
5 changes: 2 additions & 3 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/fsnotify/fsnotify v1.5.1
github.com/go-logr/logr v1.2.0
github.com/go-logr/zapr v1.2.0
github.com/onsi/ginkgo v1.16.5
github.com/onsi/ginkgo/v2 v2.1.1
Copy link
Member

Choose a reason for hiding this comment

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

Is not it a breaking change?
Will not this change affects who is using C+R and testing their projects with ENV TEST for example?
Will not be required to migrate the code to do the same with v2 as well?

Copy link
Member

Choose a reason for hiding this comment

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

No. The package is only used in tests. All changed files are also _test.go files and those can't be imported by other packages/programs. So there is no way this would affect consumers of controller-runtime.

The dependency in go.mod also doesn't seem to have an effect, at least cluster-api tests are still executing fine when using a replace directive.

Copy link
Member

Choose a reason for hiding this comment

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

@schrej,

Controller runtime provides EnvTest, which allows us to write the tests.
It seems that it is a breaking change ( changes the behaviour for who is importing kubebuilder and writing tests with. Am I right? if yes, I think we need to clarify that in the title as ⚠️ at least. )

Copy link
Member

Choose a reason for hiding this comment

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

Ah, you are right. There actually is one package that does import ginkgo outside of tests: pkg/envtest/printer
But it seems like the v2 suffix of the new version allows to import both versions at the same time. At least the compiler didn't complain when I used this PR with cluster-api, and that one is still on v1.

Copy link
Member

Choose a reason for hiding this comment

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

Note that the ginkgo made a MAJOR bump. That is only required when we have breaking changes ( unless you have a version that is 0.MINOR releases which means initial development )

The MAJOR bump occurred because they removed some deprecated options.

Therefore, consumers of controller runtime might be impacted because they will need to update and change their tests. So, we need to update the title of this PR ( which is used for generating the release notes ) with ⚠️ instead of use ✨. Also, it would be nice to have in the description of the PR the link for the release notes of the dep as the guidance to upgrade from v1 to v2.

github.com/onsi/gomega v1.18.1
github.com/prometheus/client_golang v1.12.1
github.com/prometheus/client_model v0.2.0
Expand Down Expand Up @@ -52,7 +52,7 @@ require (
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/nxadm/tail v1.4.8 // indirect
github.com/onsi/ginkgo v1.16.5 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/prometheus/common v0.32.1 // indirect
github.com/prometheus/procfs v0.7.3 // indirect
Expand All @@ -66,7 +66,6 @@ require (
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/protobuf v1.27.1 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
k8s.io/kube-openapi v0.0.0-20220328201542-3ee0da9b0b42 // indirect
Expand Down
3 changes: 2 additions & 1 deletion go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,9 @@ github.com/onsi/ginkgo v1.14.0/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9k
github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0=
github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE=
github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU=
github.com/onsi/ginkgo/v2 v2.0.0 h1:CcuG/HvWNkkaqCUpJifQY8z7qEMBJya6aLPx6ftGyjQ=
github.com/onsi/ginkgo/v2 v2.0.0/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c=
github.com/onsi/ginkgo/v2 v2.1.1 h1:LCnPB85AvFNr91s0B2aDzEiiIg6MUwLYbryC1NSlWi8=
github.com/onsi/ginkgo/v2 v2.1.1/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c=
github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA=
github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
Expand Down
13 changes: 9 additions & 4 deletions pkg/builder/builder_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package builder
import (
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/meta"
Expand All @@ -36,10 +36,11 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook"
)

const suiteName = "application Suite"

func TestBuilder(t *testing.T) {
RegisterFailHandler(Fail)
suiteName := "application Suite"
RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)})
RunSpecs(t, suiteName)
}

var testenv *envtest.Environment
Expand All @@ -63,7 +64,7 @@ var _ = BeforeSuite(func() {

webhook.DefaultPort, _, err = addr.Suggest("")
Expect(err).NotTo(HaveOccurred())
}, 60)
acumino marked this conversation as resolved.
Show resolved Hide resolved
})

var _ = AfterSuite(func() {
Expect(testenv.Stop()).To(Succeed())
Expand All @@ -75,6 +76,10 @@ var _ = AfterSuite(func() {
webhook.DefaultPort = 9443
})

var _ = ReportAfterSuite("Report to Prow", func(report Report) {
printer.AddReport(report, suiteName)
})

func addCRDToEnvironment(env *envtest.Environment, gvks ...schema.GroupVersionKind) {
for _, gvk := range gvks {
plural, singular := meta.UnsafeGuessKindToResource(gvk)
Expand Down
7 changes: 3 additions & 4 deletions pkg/builder/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"sync/atomic"

"github.com/go-logr/logr"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -321,7 +321,7 @@ var _ = Describe("application", func() {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
doReconcileTest(ctx, "3", m, false, bldr)
}, 10)
})

It("should Reconcile Watches objects", func() {
m, err := manager.New(cfg, manager.Options{})
Expand All @@ -336,7 +336,7 @@ var _ = Describe("application", func() {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
doReconcileTest(ctx, "4", m, true, bldr)
}, 10)
})
})

Describe("Set custom predicates", func() {
Expand Down Expand Up @@ -552,7 +552,6 @@ func doReconcileTest(ctx context.Context, nameSuffix string, mgr manager.Manager
go func() {
defer GinkgoRecover()
Expect(mgr.Start(ctx)).NotTo(HaveOccurred())
By("Stopping the application")
}()

By("Creating a Deployment")
Expand Down
2 changes: 1 addition & 1 deletion pkg/builder/webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"os"
"strings"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down
13 changes: 9 additions & 4 deletions pkg/cache/cache_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package cache_test
import (
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
Expand All @@ -29,10 +29,11 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)

const suiteName = "Cache Suite"

func TestSource(t *testing.T) {
RegisterFailHandler(Fail)
suiteName := "Cache Suite"
RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)})
RunSpecs(t, suiteName)
}

var testenv *envtest.Environment
Expand All @@ -50,8 +51,12 @@ var _ = BeforeSuite(func() {

clientset, err = kubernetes.NewForConfig(cfg)
Expect(err).NotTo(HaveOccurred())
}, 60)
})

var _ = AfterSuite(func() {
Expect(testenv.Stop()).To(Succeed())
})

var _ = ReportAfterSuite("Report to Prow", func(report Report) {
printer.AddReport(report, suiteName)
})
11 changes: 5 additions & 6 deletions pkg/cache/cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ import (
"sort"
"strconv"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -1540,7 +1539,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca

By("verifying the object is received on the channel")
Eventually(out).Should(Receive(Equal(pod)))
}, 3)
})

It("should be able to index an object field then retrieve objects by that field", func() {
By("creating the cache")
Expand Down Expand Up @@ -1590,7 +1589,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
Expect(listObj.Items).Should(HaveLen(1))
actual := listObj.Items[0]
Expect(actual.GetName()).To(Equal("test-pod-3"))
}, 3)
})

It("should allow for get informer to be cancelled", func() {
By("cancelling the context")
Expand Down Expand Up @@ -1660,7 +1659,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca

By("verifying the object's metadata is received on the channel")
Eventually(out).Should(Receive(Equal(podMeta)))
}, 3)
})

It("should be able to index an object field then retrieve objects by that field", func() {
By("creating the cache")
Expand Down Expand Up @@ -1714,7 +1713,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
Version: "v1",
Kind: "Pod",
}))
}, 3)
})

It("should allow for get informer to be cancelled", func() {
By("creating a context and cancelling it")
Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/informer_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
package cache_test

import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"k8s.io/client-go/rest"
Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/informer_cache_unit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
package cache

import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

corev1 "k8s.io/api/core/v1"
Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/internal/informers_map_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package internal
import (
"fmt"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down
11 changes: 8 additions & 3 deletions pkg/cache/internal/internal_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,18 @@ package internal
import (
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
)

const suiteName = "Cache Internal Suite"

func TestSource(t *testing.T) {
RegisterFailHandler(Fail)
suiteName := "Cache Internal Suite"
RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)})
RunSpecs(t, suiteName)
}

var _ = ReportAfterSuite("Report to Prow", func(report Report) {
printer.AddReport(report, suiteName)
})
18 changes: 11 additions & 7 deletions pkg/certwatcher/certwatcher_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,34 @@ import (
"os"
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)

var (
certPath = "testdata/tls.crt"
keyPath = "testdata/tls.key"
certPath = "testdata/tls.crt"
keyPath = "testdata/tls.key"
suiteName = "CertWatcher Suite"
)

func TestSource(t *testing.T) {
RegisterFailHandler(Fail)
suiteName := "CertWatcher Suite"
RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)})
RunSpecs(t, suiteName)
}

var _ = BeforeSuite(func() {
logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
}, 60)
})

var _ = AfterSuite(func() {
for _, file := range []string{certPath, keyPath} {
_ = os.Remove(file)
}
}, 60)
})

var _ = ReportAfterSuite("Report to Prow", func(report Report) {
printer.AddReport(report, suiteName)
})
2 changes: 1 addition & 1 deletion pkg/certwatcher/certwatcher_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"os"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/prometheus/client_golang/prometheus/testutil"
"sigs.k8s.io/controller-runtime/pkg/certwatcher"
Expand Down
13 changes: 9 additions & 4 deletions pkg/client/apiutil/apiutil_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package apiutil
import (
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
Expand All @@ -28,10 +28,11 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)

const suiteName = "API Utilities Test Suite"

func TestSource(t *testing.T) {
RegisterFailHandler(Fail)
suiteName := "API Utilities Test Suite"
RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)})
RunSpecs(t, suiteName)
}

var cfg *rest.Config
Expand All @@ -41,4 +42,8 @@ var _ = BeforeSuite(func() {

// for things that technically need a rest.Config for defaulting, but don't actually use them
cfg = &rest.Config{}
}, 60)
})

var _ = ReportAfterSuite("Report to Prow", func(report Report) {
printer.AddReport(report, suiteName)
})
2 changes: 1 addition & 1 deletion pkg/client/apiutil/dynamicrestmapper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"fmt"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/onsi/gomega/format"
"github.com/onsi/gomega/types"
Expand Down
13 changes: 9 additions & 4 deletions pkg/client/client_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package client_test
import (
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/scheme"
Expand All @@ -32,10 +32,11 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)

const suiteName = "Client Suite"

func TestSource(t *testing.T) {
RegisterFailHandler(Fail)
suiteName := "Client Suite"
RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)})
RunSpecs(t, suiteName)
}

var testenv *envtest.Environment
Expand All @@ -55,8 +56,12 @@ var _ = BeforeSuite(func() {
Expect(err).NotTo(HaveOccurred())

Expect(pkg.AddToScheme(scheme.Scheme)).NotTo(HaveOccurred())
}, 60)
})

var _ = AfterSuite(func() {
Expect(testenv.Stop()).To(Succeed())
})

var _ = ReportAfterSuite("Report to Prow", func(report Report) {
printer.AddReport(report, suiteName)
})
Loading