Imagestream and Buildconfigs unit tests #478

ashetty1 · 2018-05-28T06:49:12Z

Adds test for NewAppS2I, GetImageStreams func in pkg/occlient/occlient.go

ashetty1 · 2018-05-30T12:30:42Z

Up for the first review @cdrage @kadel @syamgk

pkg/occlient/occlient_test.go

 )

+// fakeImageStream gets imagestream for the reactor
+func fakeImageStream() *imagev1.ImageStream {


cdrage

This is a ton of tests being added!

Many thanks. This LGTM!

surajnarwade

Tested this PR, works && LGTM :)

syamgk · 2018-06-14T05:59:26Z

LGTM

kadel · 2018-06-18T10:24:29Z

pkg/occlient/occlient_test.go

+				tt.args.labels,
+				tt.args.annotations)
+
+			if (err != nil) != tt.wantErr {


It should do more than just check if the error was returned.
We are already doing this in the code.

The test should verify that odo component or openshift objects were created.

kadel · 2018-06-18T10:25:10Z

pkg/occlient/occlient_test.go

+
+			if (err != nil) != tt.wantErr {
+				t.Errorf("NewAppS2I() error = %#v, wantErr %#v", err, tt.wantErr)
+				return


return shouldn't be there, otherwise, it won't continue to the next test case.

kadel · 2018-06-18T10:27:57Z

pkg/occlient/occlient_test.go

+		{
+			name: "testing a valid imagestream",
+			args: args{
+				namespace: "testing",


testing namespace is passed, but in want namespace is openshift that looks strange

kadel · 2018-06-18T10:28:30Z

pkg/occlient/occlient_test.go

+		args      args
+		wantErr   bool
+	}{
+		{


there should be more than one testcase

Hmm, it is getting imagestreams. Any suggestion on what the second test could be?

tests should also verify that error is returned when it should be returned

builderImage that doesn't exist

empty builderImage

verify that if empty gitURL is passed that BuildConfig has BuildSourceBinary

verify that if gitURL is not empty than BuildConfig has GitBuildSource
...
...

@kadel sure! Was wondering if empty gitUrl or builderImage would be a valid usecase for us. Thanks for the clarification. Will add that to this as well as other functions.

One important test that I think we are still missing is non-empty builderImage that doesn't' exist.

@kadel how to do that? You mean not have a valid response through reactor?

kadel · 2018-06-18T10:28:46Z

pkg/occlient/occlient_test.go

+		want    []imagev1.ImageStream
+		wantErr bool
+	}{
+		{


there should be more than one test case

kadel · 2018-06-18T10:33:50Z

pkg/occlient/occlient_test.go

+		wantGit     *buildv1.BuildSource
+		wantErr     bool
+	}{
+		{


there should be more test cases, for example one with gitUrl: ""

kadel · 2018-06-18T10:35:40Z

pkg/occlient/occlient_test.go

+				"app.kubernetes.io/url":                   "https://github.com/openshift/ruby",
+				"app.kubernetes.io/component-source-type": "git",
+			},
+			wantGit: &buildv1.BuildSource{


wantBuildSource might be a better name

kadel · 2018-07-11T08:31:43Z

pkg/occlient/occlient_test.go

+			}
+
+			if err == nil {
+				createdIS := fkclientset.ImageClientset.Actions()[2].(ktesting.CreateAction).GetObject().(*imagev1.ImageStream)


why are we checking just 3rd action? What if the order in which ImageStream is created changes?

ashetty1 · 2018-07-16T11:08:42Z

The tests are failing because our functions don't have checks for empty strings. @kadel, kindly check if this looks okay

…ons in pkg/occlient/occlient.go: Imagestream functions covered: NewAppS2I(), GetImageStreams() Buildconfig functions covered: StartBuild() Watch functions covered: WaitForBuildToFinish(), WaitAndGetPod() project funtions: CreateNewProject()

ashetty1 · 2018-07-25T10:48:45Z

@cdrage those failing tests have been commented now.

ashetty1 force-pushed the is_test branch from 6390083 to 8e061ce Compare May 28, 2018 06:52

surajnarwade added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label May 29, 2018

ashetty1 force-pushed the is_test branch 2 times, most recently from 3d3eaab to b4030b8 Compare May 30, 2018 09:26

ashetty1 changed the title ~~[WIP] Imagestream unit tests~~ Imagestream unit tests May 30, 2018

ashetty1 force-pushed the is_test branch 2 times, most recently from d386f46 to 1d633b1 Compare May 30, 2018 12:13

syamgk reviewed Jun 4, 2018

View reviewed changes

pkg/occlient/occlient_test.go Outdated

)

// fakeImageStream gets imagestream for the reactor

func fakeImageStream() *imagev1.ImageStream {

This comment was marked as resolved.

Sign in to view

ashetty1 changed the title ~~Imagestream unit tests~~ Imagestream and Buildconfigs unit tests Jun 5, 2018

ashetty1 force-pushed the is_test branch from 42f097b to 3541b38 Compare June 7, 2018 11:54

surajnarwade removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Jun 11, 2018

ashetty1 force-pushed the is_test branch from 3541b38 to b05e849 Compare June 13, 2018 10:49

cdrage approved these changes Jun 13, 2018

View reviewed changes

surajnarwade approved these changes Jun 14, 2018

View reviewed changes

ashetty1 force-pushed the is_test branch from b8ce00f to 646fbdc Compare June 14, 2018 12:28

kadel requested changes Jun 18, 2018

View reviewed changes

ashetty1 added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. labels Jun 19, 2018

ashetty1 force-pushed the is_test branch 2 times, most recently from 59a7e87 to 6db3649 Compare July 3, 2018 08:04

ashetty1 mentioned this pull request Jul 4, 2018

Fixes odo update from git to binary/local and vice versa #518

Merged

ashetty1 removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. labels Jul 4, 2018

ashetty1 force-pushed the is_test branch 3 times, most recently from 2c58bf9 to 39b7fad Compare July 10, 2018 12:27

kadel reviewed Jul 11, 2018

View reviewed changes

ashetty1 force-pushed the is_test branch from 39b7fad to 80df4e4 Compare July 16, 2018 09:07

kadel mentioned this pull request Jul 18, 2018

Add empty image name validation #586

Merged

cdrage mentioned this pull request Jul 18, 2018

Show tags / versions when doing odo catalog list #558

Merged

ashetty1 force-pushed the is_test branch 2 times, most recently from b8d3cf6 to ba71fee Compare July 23, 2018 11:27

ashetty1 force-pushed the is_test branch from ba71fee to e5cf718 Compare July 25, 2018 09:26

ashetty1 mentioned this pull request Jul 25, 2018

Add validation for empty values in pkg/occlient/occlient.go #598

Closed

kadel approved these changes Jul 25, 2018

View reviewed changes

cdrage merged commit c501b68 into redhat-developer:master Jul 25, 2018

snyk-bot mentioned this pull request Aug 13, 2021

[Snyk] Upgrade css-what from 5.0.0 to 5.0.1 adamkeinan/odo#2

Open

This was referenced Sep 30, 2022

[Snyk] Fix for 16 vulnerabilities adamkeinan/odo#50

Open

[Snyk] Fix for 16 vulnerabilities adamkeinan/odo#51

Open

[Snyk] Fix for 16 vulnerabilities adamkeinan/odo#52

Open

adamkeinan mentioned this pull request Oct 7, 2022

[Snyk] Fix for 16 vulnerabilities adamkeinan/odo#53

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Imagestream and Buildconfigs unit tests #478

Imagestream and Buildconfigs unit tests #478

ashetty1 commented May 28, 2018 •

edited

Loading

ashetty1 commented May 30, 2018

This comment was marked as resolved.

cdrage left a comment

surajnarwade left a comment

syamgk commented Jun 14, 2018

kadel Jun 18, 2018

kadel Jun 18, 2018

kadel Jun 18, 2018

kadel Jun 18, 2018

ashetty1 Jul 3, 2018

kadel Jul 11, 2018 •

edited

Loading

ashetty1 Jul 11, 2018

kadel Jul 19, 2018

ashetty1 Jul 20, 2018

kadel Jun 18, 2018

kadel Jun 18, 2018

kadel Jun 18, 2018

kadel Jul 11, 2018

ashetty1 commented Jul 16, 2018

ashetty1 commented Jul 25, 2018

Imagestream and Buildconfigs unit tests #478

Imagestream and Buildconfigs unit tests #478

Conversation

ashetty1 commented May 28, 2018 • edited Loading

ashetty1 commented May 30, 2018

This comment was marked as resolved.

cdrage left a comment

Choose a reason for hiding this comment

surajnarwade left a comment

Choose a reason for hiding this comment

syamgk commented Jun 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kadel Jul 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashetty1 commented Jul 16, 2018

ashetty1 commented Jul 25, 2018

ashetty1 commented May 28, 2018 •

edited

Loading

kadel Jul 11, 2018 •

edited

Loading