Skip to content

Unit tests for init.go #24

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
Merged
merged 1 commit into from
Jul 22, 2021
Merged

Unit tests for init.go #24

merged 1 commit into from
Jul 22, 2021

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

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

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

/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

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

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