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

bugfix: Fixes timer replacement without clearing the old one present #141

Merged
merged 1 commit into from
Mar 6, 2023

Conversation

rkilingr
Copy link
Contributor

@rkilingr rkilingr commented Mar 6, 2023

Fixes issue #??
#140

Proposed Changes

  • Add a Timer Stop before replacing the old timer with the rescheduled one.

Testing Done

  • No increase seen even after days
    image

  • Unit tests

fetching envtest tools@1.19.2 (into '/Users/rkilingar/projects/active-monitor/testbin')
tar: Failed to set default locale
x bin/
x bin/etcd
x bin/kubectl
x bin/kube-apiserver
setting up env vars
ok      github.com/keikoproj/active-monitor/api/v1alpha1        17.629s coverage: 0.8% of statements
ok      github.com/keikoproj/active-monitor/controllers 42.703s coverage: 66.5% of statements
ok      github.com/keikoproj/active-monitor/metrics     2.106s  coverage: 81.8% of statements
?       github.com/keikoproj/active-monitor/store       [no test files]

@rkilingr rkilingr marked this pull request as ready for review March 6, 2023 06:04
@rkilingr rkilingr requested a review from a team as a code owner March 6, 2023 06:04
@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Merging #141 (710b941) into master (2c2375e) will increase coverage by 1.51%.
The diff coverage is 0.00%.

❗ Current head 710b941 differs from pull request most recent head 505d48b. Consider uploading reports for the commit 505d48b to get more accurate results

@@            Coverage Diff             @@
##           master     #141      +/-   ##
==========================================
+ Coverage   61.42%   62.93%   +1.51%     
==========================================
  Files           3        3              
  Lines         910      912       +2     
==========================================
+ Hits          559      574      +15     
+ Misses        263      250      -13     
  Partials       88       88              
Impacted Files Coverage Δ
controllers/healthcheck_controller.go 62.66% <0.00%> (+1.55%) ⬆️

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

Signed-off-by: RaviKiran Kilingar <ravikirankilingar@gmail.com>
@rkilingr
Copy link
Contributor Author

rkilingr commented Mar 6, 2023

Updated unit tests to include the diff

image

@RaviHari RaviHari merged commit 0c55063 into keikoproj:master Mar 6, 2023
@rkilingr rkilingr deleted the 140-fix-timer branch March 6, 2023 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants