-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[#6712] Use HostnameEndpoint with Agent for HTTPS #624
Conversation
git-svn-id: svn://svn.twistedmatrix.com/svn/Twisted/branches/hostagent-6712@46787 bbbe8e31-12d6-0310-92fd-ac37d47ddeeb
the tests are somewhat fragile though, so they fail git-svn-id: svn://svn.twistedmatrix.com/svn/Twisted/branches/hostagent-6712@46788 bbbe8e31-12d6-0310-92fd-ac37d47ddeeb
this removes some fragility from the tests
This is probably going to fail the nodeps builders, since |
Current coverage is 91.04% (diff: 100%)@@ trunk #624 diff @@
==========================================
Files 837 837
Lines 146006 146069 +63
Methods 0 0
Messages 0 0
Branches 12910 12912 +2
==========================================
- Hits 133033 132990 -43
- Misses 10729 10815 +86
- Partials 2244 2264 +20
|
In general looks good (I skimmed the tests a bit more than earlier code). However, since the goal was to make Agent work with IPv6, perhaps a test that actually does that should be added? |
Thanks @itamarst ! Since you didn't specify a "next action" I'm going to interpret this as "add a test that verifies IPv6 connectivity for Agent, then land". The reason I didn't begin with such a test is that, as it turns out, there's no new code here specifically for IPv6, so there was no opportunity to write a test that failed. Previously there was no way to mock out name resolution, and once name resolution is mocked out, the handling of an IPv6 string vs. an IPv4 string is entirely internal to That said, those are just reasons that a unit test was not required in TDD discipline; a little integration test to make sure nothing blows up once an |
I _thought_ that it was using HostnameEndpoint for plain-text HTTP before, but it turns out that this branch is what changes that as well.
(It did seem like TDD would be tricky, yeah, since code changed so much.) |
https://twistedmatrix.com/trac/ticket/6712