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

test_keepalive_ping_with_no_ping_timeout fails on x86_64 #1026

Closed
mcepl opened this issue Aug 10, 2021 · 9 comments
Closed

test_keepalive_ping_with_no_ping_timeout fails on x86_64 #1026

mcepl opened this issue Aug 10, 2021 · 9 comments

Comments

@mcepl
Copy link

mcepl commented Aug 10, 2021

When packaging this package for openSUSE (version 9.1) we have test test_keepalive_ping_with_no_ping_timeout failing, but just intermittently on x86_64 (test passes perfectly on other platforms):

[   47s] ======================================================================
[   47s] ERROR: test_keepalive_ping_with_no_ping_timeout (tests.legacy.test_protocol.ServerTests)
[   47s] ----------------------------------------------------------------------
[   47s] Traceback (most recent call last):
[   47s]   File "/home/abuild/rpmbuild/BUILD/websockets-9.1/tests/legacy/test_protocol.py", line 1202, in test_keepalive_ping_with_no_ping_timeout
[   47s]     ping_1_again, ping_2 = tuple(self.protocol.pings)
[   47s] ValueError: too many values to unpack (expected 2)
[   47s] 
[   47s] ----------------------------------------------------------------------

Complete build log with versions of packages used and steps taken.

@aaugustin
Copy link
Member

Yes, I saw that before, the test is flaky. This problem will be resolved when I deprecate the legacy module and remove the corresponding tests. If it's a problem for you, please submit a pull request. You get paid for this; I don't ;-)

@aaugustin
Copy link
Member

The ETA for deprecating the legacy module is about three years from now.

@aaugustin
Copy link
Member

Setting the WEBSOCKETS_TESTS_TIMEOUT_FACTOR environment variable to 10 or 100 could help too.

@mcepl
Copy link
Author

mcepl commented Aug 11, 2021

Setting the WEBSOCKETS_TESTS_TIMEOUT_FACTOR environment variable to 10 or 100 could help too.

Right, this helped. Thank you. We had it at 5 previously, but that's apparently not enough. 10 seems to be.

You get paid for this; I don't ;-)

Right, me and two and half person are maintaining around three thousand Python packages in couple of *SUSE distributions. And let us not forget about the interpreter itself. ;)

@aaugustin
Copy link
Member

I suppose one of these two scenarios is true:

  • your employer engaged into an endeavor (packaging 3k Python packages instead of letting customers to pip install them) that has no business value; they cannot make enough money to pay enough maintainers; in that case, maybe just stop packaging all these packages?
  • your employer is actually making money, but chooses to pay you to report bugs to unpaid maintainers of open-source software; in that case, maybe just stop reporting bugs?

Of course I'm aware that you're just trying to get your job done and you aren't calling the shots here.


That said, I have one suggestion that may be in your power. Since you don't have the capacity to act on broken tests, why do you bother running tests and reporting issues when they fail?

Maybe you could trust that I run them — see GitHub Actions — and that I'm not going to release crap. I could also release crap that isn't exercised by the tests and you wouldn't see anything...

SUSE running tests only results in filing issues that don't create value for you (because I won't fix them) nor for me (because I'd rather people use pip install than distro packages); this doesn't seem useful!

@mcepl
Copy link
Author

mcepl commented Aug 11, 2021

Wow! Somebody is grumpy. ;)

You obviously consider providers of enterprise distributions to be just leeches, employing people just to resolve their problems by filing reports upstream. It is lovely how perfect and solid opinion you have made without research of what the reality is.

Just in the last couple of months we three have send couple of hundreds patches upstream dealing with various issues. And yes, in case, when the problem is too complicated, we may ask for help upstream, because we think that upstream developers may have better knowledge of deeper issues with the code.

Concerning running tests in our builds. Why do you think we do so? Because we find all those bugs which need to be patched all the time. Of course, it cannot happen to your project, because you are so perfect that your software cannot have any bugs, but for those packages other developers find our help useful.

@aaugustin
Copy link
Member

aaugustin commented Aug 11, 2021

Well, for taking the time to report this bug, and apologies if the conversation degenerated quickly.

FWIW I've been on both sides of the deal — managing hundreds of engineers & inheriting contracts worth hundreds of k€ / year for corporate Linux distros on one side; maintainer of open source projects packaged by the same corporate Linux distros on the other side — but that's just one personal experience, so, anecdote more than data.

Good to hear that you're actually able to submit patches for important bugs! Keep up the good work :-)

@mcepl
Copy link
Author

mcepl commented Aug 11, 2021

Good to hear that you're actually able to submit patches for important bugs! Keep up the good work :-)

I don’t know where you worked, but before working at SUSE I was for eleven years with Red Hat, and in both places “Upstream first” was not only allowed, but required, and in Red Hat engineers have number of upstreamed patches as part of their evaluation (SUSE is probably less strict, but if we are keeping too many patches inside, we are talked to by our superiors as well).

@aaugustin
Copy link
Member

I worked at a customer of Linux distros, not a provider :-)

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this issue Aug 12, 2021
https://build.opensuse.org/request/show/911402
by user mcepl + RBrownSUSE
- Increase WEBSOCKETS_TESTS_TIMEOUT_FACTOR to 10 to make
  test_keepalive_ping_with_no_ping_timeout pass again.
- Remove skip-test_keepalive_ping_with_no_ping_timeout.patch, of course.
- Use %pyunittest macro instead of doing it on our own.
- Add skip-test_keepalive_ping_with_no_ping_timeout.patch for
  gh#python-websockets/websockets#1026.
kxxt added a commit to kxxt/archriscv-packages that referenced this issue Jun 14, 2023
- Allow the timeout factor to be float: python-websockets/websockets#1371
  - The patch needs to be stored in our repo because the patch of the PR
    won't apply to v10.4.
- Set timeout factor to 1.1 to make the tests pass on riscv64 boards.
  (As suggested in
  python-websockets/websockets#1026)
felixonmars pushed a commit to felixonmars/archriscv-packages that referenced this issue Jun 14, 2023
- Allow the timeout factor to be float: python-websockets/websockets#1371
  - The patch needs to be stored in our repo because the patch of the PR
    won't apply to v10.4.
- Set timeout factor to 1.1 to make the tests pass on riscv64 boards.
  (As suggested in
  python-websockets/websockets#1026)
aaugustin added a commit that referenced this issue Sep 22, 2024
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

No branches or pull requests

2 participants