Mock DNS package, extend testing (1/3) #630
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kuritka
requested review from
donovanmuller,
jkremser,
k0da,
somaritane and
ytsarev
as code owners
decided to expand the tests for the DNS package (the current coverage is only 20%). Consequence: - I had to split the creation of ObjectManager and infoblox provider - FakeInfoblox client is no longer needed - we have mocks (cleaner design separates test code from execution code) - I don't need to keep the Overrides.FakeInfoblox switch in the config - I'll remove it in the next PR - We can test more - in the next PR I will extend the DNS package testing - I moved functions `checkZoneDelegated`, `filterOutDelegateTo` from `ib-client.go` to `infoblox-client.go`. IBX provider constructor accepts ObjectManager interface now so I had to extend factory and fix several tests. To test the change, I created one test `TestCreateZoneDelegationForExternalDNS` and generated the necessary mocks. We currently have around 40% coverage but as said, will add new tests in following PR. Signed-off-by: kuritka <[email protected]>
kuritka
force-pushed
the
ibx-provider-split2
branch
from
c1c2178 to
cf72e5e
Compare
kuritka
changed the title
kuritka
added a commit
that referenced
this pull request
Sep 30, 2021
This is a follow-up PR to #630 (1/3), do not review until #630 is moderated Everything is now being mocked up, there is no need to distinguish between local and production infoblox. Bool switch no longer makes sense, removing Signed-off-by: kuritka <[email protected]>
kuritka
added a commit
that referenced
this pull request
Sep 30, 2021
This PR is dependent on #630, please review this PR when #630 is merged. Thanks to the redesign and mock I can test what is going on inside the big functions in the DNS package. I have written several tests that create/update the DNS record and assert values at runtime (not just the error the function returns). I managed to achieve 62% coverage. The rest are mostly constructor functions, single-line functions that call the assistant (this is more about testing the assistant package). Signed-off-by: kuritka <[email protected]>
kuritka
added a commit
that referenced
this pull request
Sep 30, 2021
This is a follow-up PR to #630 (1/3), do not review until #630 is merged Everything is now being mocked up, there is no need to distinguish between local and production infoblox. Bool switch no longer makes sense, removing Signed-off-by: kuritka <[email protected]>
kuritka
added a commit
that referenced
this pull request
Sep 30, 2021
This is a follow-up PR to #630 (1/3), do not review until #630 is merged. Thanks to the ntroducing mocks I can test what is going on inside the large functions in the DNS package. I have written several tests that create/update the DNS record and assert values at runtime (not just asserting the error the function returns). I managed to achieve 62% coverage. The uncovered are mostly constructor functions, single-line functions that call the assistant package directly (this is more about testing the assistant package). Signed-off-by: kuritka <[email protected]>
kuritka
added a commit
that referenced
this pull request
Sep 30, 2021
This is a follow-up PR to #630 (1/3), do not review until #630 is merged. Thanks to the ntroducing mocks I can test what is going on inside the large functions in the DNS package. I have written several tests that create/update the DNS record and assert values at runtime (not just asserting the error the function returns). I managed to achieve 62% coverage. The uncovered are mostly constructor functions, single-line functions that call the assistant package directly (this is more about testing the assistant package). Signed-off-by: kuritka <[email protected]>
ytsarev
approved these changes
Oct 2, 2021
kuritka
added a commit
that referenced
this pull request
Oct 2, 2021
This is a follow-up PR to #630 (1/3), do not review until #630 is merged. Thanks to the ntroducing mocks I can test what is going on inside the large functions in the DNS package. I have written several tests that create/update the DNS record and assert values at runtime (not just asserting the error the function returns). I managed to achieve 62% coverage. The uncovered are mostly constructor functions, single-line functions that call the assistant package directly (this is more about testing the assistant package). Signed-off-by: kuritka <[email protected]>
kuritka
added a commit
that referenced
this pull request
Oct 2, 2021
This is a follow-up PR to #630 (1/3), do not review until #630 is merged Everything is now being mocked up, there is no need to distinguish between local and production infoblox. Bool switch no longer makes sense, removing Signed-off-by: kuritka <[email protected]>
kuritka
added a commit
that referenced
this pull request
Oct 2, 2021
This is a follow-up PR to #630 (1/3), do not review until #630 is merged Everything is now being mocked up, there is no need to distinguish between local and production infoblox. Bool switch no longer makes sense, removing Signed-off-by: kuritka <[email protected]>
kuritka
added a commit
that referenced
this pull request
Oct 4, 2021
This is a follow-up PR to #630 (1/3), do not review until #630 is merged. Thanks to the ntroducing mocks I can test what is going on inside the large functions in the DNS package. I have written several tests that create/update the DNS record and assert values at runtime (not just asserting the error the function returns). I managed to achieve 62% coverage. The uncovered are mostly constructor functions, single-line functions that call the assistant package directly (this is more about testing the assistant package). Signed-off-by: kuritka <[email protected]>
kuritka
added a commit
that referenced
this pull request
Oct 4, 2021
This is a follow-up PR to #630 (1/3), do not review until #630 is merged. Thanks to the ntroducing mocks I can test what is going on inside the large functions in the DNS package. I have written several tests that create/update the DNS record and assert values at runtime (not just asserting the error the function returns). I managed to achieve 62% coverage. The uncovered are mostly constructor functions, single-line functions that call the assistant package directly (this is more about testing the assistant package). Signed-off-by: kuritka <[email protected]>
kuritka
added a commit
that referenced
this pull request
Oct 4, 2021
This is a follow-up PR to #630 (1/3), do not review until #630 is merged. Thanks to the ntroducing mocks I can test what is going on inside the large functions in the DNS package. I have written several tests that create/update the DNS record and assert values at runtime (not just asserting the error the function returns). I managed to achieve 62% coverage. The uncovered are mostly constructor functions, single-line functions that call the assistant package directly (this is more about testing the assistant package). Signed-off-by: kuritka <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
decided to expand the tests for the DNS package (the current coverage is only 20%).
Consequence:
checkZoneDelegated,filterOutDelegateTofromib-client.gotoinfoblox-client.go.IBX provider constructor accepts ObjectManager interface now so I had to extend factory and fix several tests.
To test the change, I created one test
TestCreateZoneDelegationForExternalDNSand generated the necessary mocks.We currently have around 40% coverage but as said, will add new tests in following PR (3/3).
Signed-off-by: kuritka [email protected]