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

Fix intermittent test false negative #982

Merged
merged 17 commits into from
Jun 16, 2021
Merged

Fix intermittent test false negative #982

merged 17 commits into from
Jun 16, 2021

Conversation

yvanoers
Copy link
Collaborator

This fixes the test of processFilteredNodes that sometimes failed.
This became a large refactor:

  • processFilteredNodes no longer exists and is replaced by several smaller functions
  • significantly revised test code
  • allows for injecting the selector, which paves the way for e.g. selecting nodes based on weight
  • some code improvements and markup revisions I came across

@yvanoers yvanoers requested a review from vcastellm June 11, 2021 22:14
vcastellm
vcastellm requested changes Jun 14, 2021
View reviewed changes
cmd/agent.go Show resolved Hide resolved
dkron/run.go Outdated Show resolved Hide resolved
vcastellm
vcastellm requested changes Jun 15, 2021
View reviewed changes
Copy link
Member

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

I wouldn't remove any other public method, they could be in use by third parties.

dkron/agent.go Outdated Show resolved Hide resolved
dkron/agent.go Outdated Show resolved Hide resolved
@yvanoers
Revert "Remove unused code"
5329cf8 
This reverts commit 19f19cd.
@yvanoers
Copy link
Collaborator Author

Whoops, didn't realize, sorry. I reverted the removal of seemingly unused code.

vcastellm
vcastellm approved these changes Jun 16, 2021
View reviewed changes
Copy link
Member

@vcastellm vcastellm left a comment

Choose a reason for hiding this comment

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

LGTM

@vcastellm vcastellm merged commit 1ebf862 into distribworks:master Jun 16, 2021
@yvanoers yvanoers deleted the inject-randomizer branch June 16, 2021 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vcastellm vcastellm vcastellm approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@yvanoers @vcastellm