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.

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

adding unit tests for broker package #983

Merged
merged 2 commits into from
Jun 14, 2018

Conversation

shawn-hurley
Copy link
Contributor

Describe what this PR does and why we need it:

Example of how to implement tests for the broker package.

Changes proposed in this pull request

  • test cases for job_state_subscriber moves to 81.7%
  • test cases for jobs moves to 70%
  • broker.go moves to 7.7%

@openshift-ci-robot openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jun 11, 2018
@shawn-hurley shawn-hurley requested review from eriknelson, jmrodri and rthallisey and removed request for eriknelson June 11, 2018 18:42
@shawn-hurley shawn-hurley added tech-debt 3.11 | release-1.3 Kubernetes 1.11 | Openshift 3.11 | Broker release-1.3 labels Jun 11, 2018
@coveralls
Copy link

coveralls commented Jun 11, 2018

Coverage Status

Coverage increased (+3.04%) to 36.245% when pulling d00ce3d on shawn-hurley:add-unit-test-broker into 9916cb0 on openshift:master.

dao: new(mocks.Dao),
config: Config{},
addExpectations: func(d *mocks.Dao) {
d.On("GetServiceInstance", u.String()).Return(nil, unknownError)
Copy link
Contributor

Choose a reason for hiding this comment

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

just trying to understand. Is this specifying the behavior when the GetServiceInstance is called or is it the expected result?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the answer is both.

https://github.com/stretchr/testify#mock-package

// TestSomething is an example of how to use our test object to
// make assertions about some target code we are testing.
func TestSomething(t *testing.T) {

  // create an instance of our test object
  testObj := new(MyMockedObject)

  // setup expectations
  testObj.On("DoSomething", 123).Return(true, nil)

  // call the code we are testing
  targetFuncThatDoesSomethingWithObj(testObj)

  // assert that the expectations were met
  testObj.AssertExpectations(t)

}

second := messages[1]
log.Infof("second: %#v", second)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to keep the log message?

}

second := messages[1]
log.Infof("second: %#v", second)
Copy link
Contributor

Choose a reason for hiding this comment

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

same might want to remove

Copy link
Contributor

@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.

broker test coverage increased from 27.9% to 32.5%

@shawn-hurley shawn-hurley merged commit bf419ad into openshift:master Jun 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 | release-1.3 Kubernetes 1.11 | Openshift 3.11 | Broker release-1.3 size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tech-debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants