Converted Repository operator tests to Mockery #461
Conversation
|
/assign @efiacor @lapentad @vjayaramrh |
|
|
||
| func (gc NephioGiteaClientMock) GetMyUserInfo() (*gitea.User, *gitea.Response, error) { | ||
| return &gitea.User{}, &gitea.Response{}, gc.myUserInfoError | ||
| type mockHelper struct { |
There was a problem hiding this comment.
Suggestion: it may be a good idea to create a separate directory called testing and defined the mockHelper struct there and then could be imported by other packages that will use mockery for unit testing
| }) | ||
| } | ||
| } | ||
|
|
||
| func initMockeryMocks(tt *repoTest) { |
There was a problem hiding this comment.
Suggestion: This function could be generalized and moved to a the testing folder as well to be reused by others.
liamfallon
force-pushed
the
convert-unit-tests-to-mockery
branch
from
53aed0c to
4bee818
Compare
liamfallon
force-pushed
the
convert-unit-tests-to-mockery
branch
from
4bee818 to
9201771
Compare
|
I need to split the mockeryutils module out into a separate PR so that it can go in first and avoid the missing ref from the reconciler. |
|
We need to wait for this PR to be merged |
liamfallon
added
documentation
|
/retest |
liamfallon
force-pushed
the
convert-unit-tests-to-mockery
branch
from
0d766e3 to
2cb7961
Compare
|
/retest |
1 similar comment
|
/retest |
|
@vjayaramrh @efiacor This is ready for review now. |
| interfaces: | ||
| GiteaClient: | ||
| config: | ||
| dir: "{{.InterfaceDir}}" |
There was a problem hiding this comment.
Not sure on the best way to fix this one but..
The mock file gets picked up by the coverage report. ie 0%
Using the config in mockery we can name the file with a _test suffix, but for me this breaks the import in the test.go.
Alternatively, which is what I plan to do for say the controller-runtime client mock, is to output to a "mocks" dir under controller/pkg and maybe exclude that in the report. Or we suffix them with _test and add the logic for that in the coverage cmd.
Thoughts?
There was a problem hiding this comment.
Can we take this discussion separately? Would it be OK to bring this PR in? I'll look at this in a subsequent PR.
There was a problem hiding this comment.
Yes, if it's not here the build fails. The main Makefile expects a Makefile in every submodule that contains "go" files.
|
/lgtm |
|
/approve |
liamfallon
force-pushed
the
convert-unit-tests-to-mockery
branch
from
c7194bf to
c1251b3
Compare
|
@vjayaramrh @efiacor Can we resurrect this work? |
|
/lgtm |
liamfallon
force-pushed
the
convert-unit-tests-to-mockery
branch
from
c1251b3 to
7e65aa2
Compare
Done |
|
@s3wong @tliron @johnbelamaric @vjayaramrh Can we get reviews on this please? |
liamfallon
changed the title
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: efiacor, henderiw The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR uses the approach and code described by @vjayaramrh in nephio-project#441 to replace the unit tests for the Repository operator. This PR replaces all the unit tests with Mockery implementations.
This PR uses the approach and code described by @vjayaramrh in #441 to replace the unit tests for the Repository operator. This PR replaces all the unit tests with Mockery implementations.