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

Tests TestSemaphoreRobust and TestSemaphoreN failed randomly #72

Closed
Loyalsoldier opened this issue Jun 25, 2021 · 3 comments
Closed

Tests TestSemaphoreRobust and TestSemaphoreN failed randomly #72

Loyalsoldier opened this issue Jun 25, 2021 · 3 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Loyalsoldier
Copy link
Contributor

See here: #68

@benja-wu benja-wu self-assigned this Jun 26, 2021
@benja-wu benja-wu added the bug Something isn't working label Jun 26, 2021
@benja-wu
Copy link
Contributor

Thx for reminding.
Let me figure out what's wrong here.

  • MacOS go test failed due to the semaphore_test
  • Ubuntu go test failed due to the cluster_test

@benja-wu benja-wu added this to To do in Easegress Project via automation Jun 26, 2021
@benja-wu benja-wu added this to the v1.0.1 milestone Jun 26, 2021
@xxx7xxxx
Copy link
Contributor

xxx7xxxx commented Jun 26, 2021

Try it multiple times, it will reproduce the failure. The reason here is it gave a too-small gap of the time in a limited CPU resource. So the failure will be eliminated by increasing the time gap. But my real proposal is to make it a benchmark test instead of unit test. It should not use time consumption to decide whether the unit test is successful. Github action apparently gives a very small resource of CPU, so it needs much longer time than our modern developing machines.

@qdongxu please check it out.

$ cd pkg/util/sem
$ GOMAXPROCS=1 go test -v
=== RUN   TestSemaphore0
--- PASS: TestSemaphore0 (0.10s)
=== RUN   TestSemaphoreRobust
--- PASS: TestSemaphoreRobust (1.07s)
=== RUN   TestSemaphoreN
    semaphore_test.go:153: time too long: 211.624608ms, sem: 799, trans: 800
--- FAIL: TestSemaphoreN (2.33s)
FAIL
exit status 1
FAIL    github.com/megaease/easegress/pkg/util/sem      3.624s

@zhao-kun zhao-kun modified the milestones: v1.0.1 , v1.0.2 Jun 29, 2021
@qdongxu qdongxu mentioned this issue Jul 5, 2021
@qdongxu
Copy link
Contributor

qdongxu commented Jul 5, 2021

We implemented a Semaphore strategy when there's not an 'official` semaphore. Now we can employ https://pkg.go.dev/golang.org/x/sync/semaphore, then less unit test is required.

Easegress Project automation moved this from To do to Done Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

5 participants