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

Fix #3016: Use scheduleWithFixedDelay for resync task execution #3031

Merged
merged 1 commit into from
Apr 30, 2021

Conversation

rohanKanojia
Copy link
Member

@rohanKanojia rohanKanojia commented Apr 21, 2021

Description

Fix #3016

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@akram
Copy link
Contributor

akram commented Apr 26, 2021

This commit b15b83f from #3017 is increasing Controller.java coverage.
This would help this PR to get merged.

@rohanKanojia rohanKanojia force-pushed the pr/issue3016 branch 3 times, most recently from 655f827 to 5f4f256 Compare April 30, 2021 11:31
@@ -248,7 +252,7 @@ void testControllerRunsReyncFunctionExpectedNumberOfTime() throws InterruptedExc
Executable controllerRun = newControllerRun(controller);
assertDoesNotThrow(controllerRun);
// We give an extra cycle to avoid clock inaccurracy interruptions
Copy link
Member

Choose a reason for hiding this comment

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

this comment seems outdated

+ When using `resyncExecutor.scheduleAtFixedRate` we're seeing inconsistent
  resync intervals since it doesn't wait for previous task to be
  terminated. Hence using `resyncExecutor.scheduleWithFixedDelay` instead.
+ Minor changes in ControllerTest regarding resync expectations/wait
  time
@sonarcloud
Copy link

sonarcloud bot commented Apr 30, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Copy link
Member

@manusa manusa left a comment

Choose a reason for hiding this comment

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

LGTM, thx!

@manusa manusa merged commit bfc3170 into fabric8io:master Apr 30, 2021
@manusa manusa mentioned this pull request Apr 30, 2021
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants