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 regression with c-ares 1.32 #200

Closed
gjasny opened this issue Jul 18, 2024 · 5 comments
Closed

Test regression with c-ares 1.32 #200

gjasny opened this issue Jul 18, 2024 · 5 comments

Comments

@gjasny
Copy link

gjasny commented Jul 18, 2024

Hello,

as you can see in the Debian Package Tracker for c-ares the upgrade from 1.31.0-1 to 1.32.2-1 introduces a regression in pycares:

 43s ======================================================================
 43s FAIL: test_idna_encoding_query_a (tests.test_all.DNSTest.test_idna_encoding_query_a)
 43s ----------------------------------------------------------------------
 43s Traceback (most recent call last):
 43s   File "/tmp/autopkgtest-lxc.awikol4c/downtmp/build.fsM/src/tests/test_all.py", line 572, in test_idna_encoding_query_a
 43s     self.assertEqual(self.errorno, pycares.errno.ARES_EBADNAME)
 43s AssertionError: 15 != 8

Debian ships a preliminary version of #194.

It looks like c-ares has changed the return code, again and it now returns ARES_ENOMEM(15) for your test case. I'd suggest to make the test more robust against any such return code changes and test for the return code to be not equal to ARES_SUCCESS.

Thanks,
Gregor

@gjasny
Copy link
Author

gjasny commented Jul 18, 2024

CC: @bradh352

@saghul
Copy link
Owner

saghul commented Jul 18, 2024

ENOMEM sounds like something went wrong, not a legit answer to a DNS query though.

@bradh352
Copy link

investigating now, i'll add this same test case to c-ares and debug

bradh352 added a commit to c-ares/c-ares that referenced this issue Jul 18, 2024
In c-ares 1.30.0 we started validating strings parsed are printable.
This caused a regression in a pycares test case due to a wrong response
code being returned as the error was being propagated from a different
section of code that was assuming the only possible failure condition
was out-of-memory.

This PR adds a fix for this and also a test case to validate it.

Ref: saghul/pycares#200
Fix By: Brad House (@bradh352)
bradh352 added a commit to c-ares/c-ares that referenced this issue Jul 18, 2024
In c-ares 1.30.0 we started validating strings parsed are printable.
This caused a regression in a pycares test case due to a wrong response
code being returned as the error was being propagated from a different
section of code that was assuming the only possible failure condition
was out-of-memory.

This PR adds a fix for this and also a test case to validate it.

Ref: saghul/pycares#200
Fix By: Brad House (@bradh352)
bradh352 added a commit to c-ares/c-ares that referenced this issue Jul 18, 2024
In c-ares 1.30.0 we started validating strings parsed are printable.
This caused a regression in a pycares test case due to a wrong response
code being returned as the error was being propagated from a different
section of code that was assuming the only possible failure condition
was out-of-memory.

This PR adds a fix for this and also a test case to validate it.

Ref: saghul/pycares#200
Fix By: Brad House (@bradh352)
bradh352 added a commit to c-ares/c-ares that referenced this issue Jul 18, 2024
In c-ares 1.30.0 we started validating strings parsed are printable.
This caused a regression in a pycares test case due to a wrong response
code being returned as the error was being propagated from a different
section of code that was assuming the only possible failure condition
was out-of-memory.

This PR adds a fix for this and also a test case to validate it.

Ref: saghul/pycares#200
Fix By: Brad House (@bradh352)
@bradh352
Copy link

alright, should be fixed

@saghul
Copy link
Owner

saghul commented Jul 18, 2024

Great! I'm closing this, since no changes are necessary in pycares.

@saghul saghul closed this as completed Jul 18, 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

3 participants