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.

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

chore: refactoring and improve coverage for K8s Sync #466

Merged
merged 5 commits into from
Mar 8, 2023
Merged

chore: refactoring and improve coverage for K8s Sync #466

merged 5 commits into from
Mar 8, 2023

Conversation

thisthat
Copy link
Member

@thisthat thisthat commented Mar 4, 2023 •
edited
Loading

This PR

  • Refactor the code to make it easier to inject the K8s clients
  • Add unit tests

Fixes #460

@codecov
Copy link

codecov bot commented Mar 4, 2023 •
edited
Loading

Codecov Report

Merging #466 (a692452) into main (5b85b2a) will increase coverage by 6.35%.
The diff coverage is 56.09%.

@@            Coverage Diff             @@
##             main     #466      +/-   ##
==========================================
+ Coverage   62.12%   68.47%   +6.35%     
==========================================
  Files          13       13              
  Lines        1650     1656       +6     
==========================================
+ Hits         1025     1134     +109     
+ Misses        562      461     -101     
+ Partials       63       61       -2
Impacted Files Coverage Δ
pkg/sync/kubernetes/kubernetes_sync.go 83.62% <56.09%> (+47.26%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@thisthat thisthat marked this pull request as ready for review March 4, 2023 08:29
@toddbaert toddbaert mentioned this pull request Mar 6, 2023
Merged
skyerus
skyerus reviewed Mar 6, 2023
View reviewed changes
pkg/sync/kubernetes/kubernetes_sync.go Outdated Show resolved Hide resolved
pkg/sync/kubernetes/kubernetes_sync_test.go Outdated Show resolved Hide resolved
pkg/sync/kubernetes/kubernetes_sync_test.go Outdated Show resolved Hide resolved
pkg/sync/kubernetes/kubernetes_sync.go Outdated Show resolved Hide resolved
@thisthat thisthat force-pushed the feat/460/improve-k8s-tests branch from 9f6bd41 to f919d00 Compare March 7, 2023 11:26
@skyerus
Copy link
Contributor

skyerus commented Mar 7, 2023

LGTM, nice :)

Kavindu-Dodan
Kavindu-Dodan approved these changes Mar 7, 2023
View reviewed changes
Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Approving. Left a minor suggestion.

Besides, I like the New function based struct construction. In furure, when all Sync providers have a similar construction, I think we should get rid of Init() function. Because, I would expect New function to construct an initialized Sync provider :)

toddbaert
toddbaert approved these changes Mar 8, 2023
View reviewed changes
Copy link
Member

@toddbaert toddbaert left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Merging... will increase coverage by 6.21%. 😎

thisthat added 5 commits March 8, 2023 08:23
@thisthat @toddbaert
small refactoring and improve coverage K8s
addcdaf 
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
@thisthat @toddbaert
add more tests
7404245 
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
@thisthat @toddbaert
improve tests
62de879 
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
@thisthat @toddbaert
add more assertions
56a2021 
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
@thisthat @toddbaert
reorder functions
a692452 
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
@toddbaert toddbaert force-pushed the feat/460/improve-k8s-tests branch from e2cdfca to a692452 Compare March 8, 2023 13:23
@toddbaert toddbaert merged commit 6dc441e into open-feature:main Mar 8, 2023
@github-actions github-actions bot mentioned this pull request Mar 8, 2023
Merged
beeme1mr pushed a commit that referenced this pull request Mar 9, 2023
@github-actions
chore(main): release 0.4.2 (#475)
5dad8ad 
🤖 I have created a release *beep* *boop*
---


##
[0.4.2](v0.4.1...v0.4.2)
(2023-03-09)


### 🧹 Chore

* Add targeted Flag to example config
([#467](#467))
([6a039ce](6a039ce))
* **deps:** pin dependencies
([#473](#473))
([679e860](679e860))
* **deps:** update google-github-actions/release-please-action digest to
e0b9d18 ([#474](#474))
([5b85b2a](5b85b2a))
* refactoring and improve coverage for K8s Sync
([#466](#466))
([6dc441e](6dc441e))


### 🐛 Bug Fixes

* add registry login
([#476](#476))
([99de755](99de755))
* **deps:** update module golang.org/x/crypto to v0.7.0
([#472](#472))
([f53f6c8](f53f6c8))
* **deps:** update module google.golang.org/protobuf to v1.29.0
([#478](#478))
([f9adc8e](f9adc8e))


### ✨ New Features

* grpc tls connectivity (grpcs)
([#477](#477))
([228f430](228f430))
* introduce per-sync configurations
([#448](#448))
([1d80039](1d80039))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skyerus skyerus skyerus left review comments

@james-milligan james-milligan james-milligan approved these changes

@toddbaert toddbaert toddbaert approved these changes

@Kavindu-Dodan Kavindu-Dodan Kavindu-Dodan approved these changes

@AlexsJones AlexsJones Awaiting requested review from AlexsJones

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Testing] Increase test coverate in K8s sync
5 participants
@thisthat @skyerus @Kavindu-Dodan @toddbaert @james-milligan