Skip to content

Unit tests for init.go#24

Merged
jmrodri merged 1 commit intooperator-framework:mainfrom
VenkatRamaraju:Init-Test
Jul 22, 2021
Merged

Unit tests for init.go#24
jmrodri merged 1 commit intooperator-framework:mainfrom
VenkatRamaraju:Init-Test

Conversation

@VenkatRamaraju
Copy link
Contributor

@VenkatRamaraju VenkatRamaraju commented Jul 6, 2021 •
edited
Loading

Added unit tests for v1alpha/init.go along with the suite tests for the v1 package.

@laxmikantbpandhare laxmikantbpandhare linked an issue Jul 6, 2021 that may be closed by this pull request
Meta-issue: Java Operator #1
Closed
26 tasks
VenkatRamaraju
VenkatRamaraju commented Jul 6, 2021
View reviewed changes
pkg/quarkus/v1alpha/init_test.go Outdated
Comment on lines 53 to 54
flagTest := pflag.NewFlagSet("testFlag", -1)
// Need clarification on what to set ErrorHandlingNumber as
Copy link
Contributor Author

@VenkatRamaraju VenkatRamaraju Jul 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Some suggestions on what to put as the "default" ErrorHandlingNumber (currently -1) would be helpful.

VenkatRamaraju
VenkatRamaraju commented Jul 6, 2021
View reviewed changes
pkg/quarkus/v1alpha/init_test.go Outdated
Comment on lines 84 to 85
testConfig, _ := config.New(config.Version{Number: 3})
// Need clarification on what to set Version number as
Copy link
Contributor Author

@VenkatRamaraju VenkatRamaraju Jul 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Here as well - suggestions on what to set as the version number would be helpful.

EDIT: Tested it out with a bunch of different values. 3 seems to be the only acceptable number.

@VenkatRamaraju
Copy link
Contributor Author

VenkatRamaraju commented Jul 6, 2021

Also, I did not write a test for the Scaffold() function (here). It only makes calls to external functions, so wasn't sure if I should trace all of them and write test cases for each one.

If you want me to dig into it and write a test for Scaffold() as well, I'll do that. Let me know.

@VenkatRamaraju VenkatRamaraju mentioned this pull request Jul 7, 2021
Merged
@VenkatRamaraju VenkatRamaraju force-pushed the Init-Test branch 3 times, most recently from 91c963f to e744bff Compare July 7, 2021 20:51
@VenkatRamaraju VenkatRamaraju mentioned this pull request Jul 8, 2021
Merged
@laxmikantbpandhare
Copy link
Member

laxmikantbpandhare commented Jul 8, 2021

Everything is working perfect.

Running Suite: v1
=================
Random Seed: 1625769325
Will run 16 of 16 specs

••••••••••••••••
Ran 16 of 16 Specs in 0.000 seconds
SUCCESS! -- 16 Passed | 0 Failed | 0 Pending | 0 Skipped
PASS

Ginkgo ran 1 suite in 1.138961774s
Test Suite Passed

@laxmikantbpandhare
Copy link
Member

laxmikantbpandhare commented Jul 8, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 8, 2021
jmrodri
jmrodri requested changes Jul 14, 2021
View reviewed changes
Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

Update the test similar to the other PR comments.

pkg/quarkus/v1alpha/init_test.go Outdated
Comment on lines 31 to 36
successInitSubcommand := &initSubcommand{domain: "testDomain"}
failureInitSubcommand := &initSubcommand{
domain: "testDomain",
projectName: "?&fail&?",
commandName: "failureTest",
}
Copy link
Member

@jmrodri jmrodri Jul 14, 2021

Choose a reason for hiding this comment

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

Put in a BeforeEach block. that way each test has a fresh version that hasn't been affected by any changes from subsequent test.

pkg/quarkus/v1alpha/init_test.go Outdated
Expect(failureInitSubcommand.commandName).NotTo(Equal(testCliMetadata.CommandName))
})

successInitSubcommand.UpdateMetadata(testCliMetadata, &testSubcommandMetadata)
Copy link
Member

@jmrodri jmrodri Jul 14, 2021

Choose a reason for hiding this comment

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

Should be part of the It

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2021
@VenkatRamaraju VenkatRamaraju requested a review from jmrodri July 14, 2021 15:28
jmrodri
jmrodri approved these changes Jul 22, 2021
View reviewed changes
Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2021
@jmrodri jmrodri merged commit 0f2c42f into operator-framework:main Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmrodri jmrodri jmrodri approved these changes

@laxmikantbpandhare laxmikantbpandhare Awaiting requested review from laxmikantbpandhare

Assignees

@jmrodri jmrodri

@VenkatRamaraju VenkatRamaraju

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Meta-issue: Java Operator

3 participants

@VenkatRamaraju @laxmikantbpandhare @jmrodri