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 unstable tests in onidle package #989

Merged
merged 2 commits into from
Jun 22, 2021
Merged

Fix unstable tests in onidle package #989

merged 2 commits into from
Jun 22, 2021

Conversation

d-uzlov
Copy link
Contributor

@d-uzlov d-uzlov commented Jun 18, 2021

Description

Issue link

fixes #988

How Has This Been Tested?

  • Added unit testing to cover
  • Tested manually
  • Tested by integration testing
  • Have not tested

Types of changes

  • Bug fix
  • New functionallity
  • Documentation
  • Refactoring
  • CI

Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
@d-uzlov d-uzlov changed the title [test] testing onidle package on CI Fix unstable tests in onidle package Jun 21, 2021
@d-uzlov d-uzlov marked this pull request as ready for review June 21, 2021 05:23
Copy link
Member

@denis-tingaikin denis-tingaikin left a comment

Choose a reason for hiding this comment

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

Please capture time on start test and wait in never, eventually time: 1s - elapsed time.

Proposal:

func TestIdleNotifier_Refresh(t *testing.T) {
now := time.Now()
...
require.Never(t, ..., time.Second - time.Since(now), testTick)
..
require.Eventually(t, ..., time.Second - time.Since(now), testTick)

}

It should prevent this kind of issue for these tests in the future.

Signed-off-by: Danil Uzlov <DanilUzlov@yandex.ru>
@denis-tingaikin denis-tingaikin merged commit ff6a8e3 into networkservicemesh:main Jun 22, 2021
@d-uzlov d-uzlov mentioned this pull request Jun 28, 2021
9 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
Development

Successfully merging this pull request may close these issues.

TestIdleNotifier_HoldingActiveRequest is unstable
2 participants