_c_urlarg.c tests should be in the core, not in web

[spiv]

_c_urlarg.c is part of the core (it is in twisted/protocols), but its unit tests are in twisted/web/test/test_http.py. Those tests should be in the core somewhere instead.

This was originally mentioned in #4142.

[spiv]

As idnar commented on IRC, another option might be to move _c_urlarg itself to web. I'm don't have a strong opinion either way.

[PenguinOfDoom]

_c_urlarg is used by mail as well as web, so core is the place for it.

[pahan]

(In [28754]) Branching to 'curlarg-tests-4162'

[Jean-Paul Calderone]

You forgot to have some revisions: ​http://twistedmatrix.com/trac/log/branches/curlarg-tests-4162

[PenguinOfDoom]

Ha ha look how dumb I am. I committed some changes to the branch now.

[jesstess]

Thanks for working on this, PenguinOfDoom. Some feedback:

• I don't know if being a C file changes how this works, but presumably _c_urlarg.py needs something like /* -*- test-case-name: twisted.protocol.test.test_c_urlarg -*- */ at the top. If that gets added, the file needs a copyright bump.
• twisted/web/test/test_http.py has pyflakes warnings about the unused urllib import and the unused transport on line 1391.
• test_http.py has one spot with trailing whitespace.
• the tests in test_c_urlarg.py need have names of the form test_myTest, and should be using assert forms instead of fail forms as per the ​test standard. The docstrings on the tests are vague and aren't actually describing the behavior we should be seeing, and testUnquote is testing multiple behaviors, which should really be split into their own tests. Can you fix these things here or open a new ticket for them?

[Thijs Triemstra]

let me know if the branch needs to be merged forward.

[Ying Li]

This looks like it's waiting for the author (PenguinOfDoom) to respond to review feedback - not sure why it was assigned to me.

[pahan]

(In [31154]) Branching to 'curlarg-eliminate-4162'

[PenguinOfDoom]

There is no credible evidence that _c_urlarg ever sped anything up, and it certainly has caused its share of problems over the years. Here's a patch to delete it!

[lvh]

Apparently there is a failing test, but I don't see how it's related...

​http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/curlarg-eliminate-4162

[PenguinOfDoom]

Oops. Fixed!

[lvh]

Looks good! Merge ftw

[pahan]

(In [31167]) Merge curlarg-eliminate-4162: Remove _c_urlarg

Author: PenguinOfDoom Reviewer: lvh Fixes: #4162

[PenguinOfDoom]

#4246 is a duplicate of this