Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Compress recursive answers if they are too big. #1307

Merged
merged 2 commits into from
Aug 17, 2015
Merged

Conversation

tomwilkie
Copy link
Contributor

Fixes #1306

@rade
Copy link
Member

rade commented Aug 16, 2015

Is it possible to write test for this that isn't horrific?

@tomwilkie
Copy link
Contributor Author

Yeah I think I could probably put together a simple-but-slightly-contrived unit test. Let me have a go.

@tomwilkie tomwilkie self-assigned this Aug 16, 2015
@tomwilkie
Copy link
Contributor Author

Actually, there seems to be know way to use the golang dns resolver with your own server - its hard wired to read /etc/resolv.conf, and I don't think rewriting that in a unit test is a good idea.

So we can have a test of the server side bits, but not the actual client error.

@tomwilkie tomwilkie changed the title Compress recursive answers if they are too big. [WIP] Compress recursive answers if they are too big. Aug 16, 2015
@tomwilkie tomwilkie changed the title [WIP] Compress recursive answers if they are too big. Compress recursive answers if they are too big. Aug 17, 2015
Tom Wilkie added 2 commits August 17, 2015 18:25
Note this doesn't test the golang resolver, where the original error
was found, as I can't find a way to point the golang resolver at a
custom dns server during a test.  It does check that a compressed
response from an upstream server is correctly handled.
@rade rade force-pushed the 1306-dns-recurse-compress branch from 5763b6d to 67f9749 Compare August 17, 2015 17:25
@rade
Copy link
Member

rade commented Aug 17, 2015

Is it possible to write test for this that isn't horrific?

Hmm. 70 lines of test code for a 3 line change.

I've fixed some minor niggles and re-based. @tomwilkie I see this is assigned to you - can I merge it?

@tomwilkie tomwilkie removed their assignment Aug 17, 2015
@tomwilkie
Copy link
Contributor Author

Yeah, test is a bit long. I can probably make it shorter with some aggressive refactoring. But I doubt its worth it.

Go ahead, merge away.

@rade rade self-assigned this Aug 17, 2015
rade added a commit that referenced this pull request Aug 17, 2015
Compress recursive answers if they are too big.

Fixes #1306.
@rade rade merged commit bbc91c1 into master Aug 17, 2015
@rade rade added this to the 1.1.0 milestone Aug 17, 2015
@rade rade deleted the 1306-dns-recurse-compress branch August 19, 2015 20:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants