Use unittest.skipIf decorator for skipping certain test functions.

[rodrigc]

https://twistedmatrix.com/trac/ticket/9812

This reduces errors reported by mypy of the form:

src/twisted/words/test/test_domish.py:193:9: error: "Callable[[ElementTests], Any]" has no attribute "skip"  [attr-defined]
            test_addContentBytes.skip = (

[rodrigc]

@wiml Could you take a look at this?

[wiml]

Whew, that's a lot of files! (Might have been easier to split it into one PR for all of the import-style changes and one for the changes that affect tests/skipping). Looks generally good to me, I have left a few comments on possible oversights and stylistic opinions in-line.

[wiml]

I don't especially like this pattern (having a pair of generically-named skip variables that are always set and used in sync). I think it'd be clearer to either use informatively-named flags (like you did with HAS_IPV6) or store the text in the skip flag (like the way this file was written before, type: Optional[str]).

[rodrigc]

In this file, the existing pattern of skipping is a bit complicated, because there are multiple reasons for skipping tests, i.e. Windows, invalid tty, pyasn1 unavailable.

I tried to keep the pattern the same while introducing the use of skipIf

[wiml]

I'm curious, is there a situation where we have IReactorSSL but not the ssl module?

[rodrigc]

I'm not sure, the tests were written like that before.

[adiroiban]

From twisted/python/_setup.py

    requirements = [
        "zope.interface >= 4.4.2",
        "constantly >= 15.1",
        "incremental >= 16.10.1",
        "Automat >= 0.3.0",
        "hyperlink >= 17.1.1",
        "PyHamcrest >= 1.9.0",
        "attrs >= 17.4.0",
    ]

pyopenssl/cryptography is not a hard requirement.

You need to do the extra [tls] install.

_EXTRA_OPTIONS = dict(
    dev=_dev,
    tls=[
        'pyopenssl >= 16.0.0',
        # service_identity 18.1.0 added support for validating IP addresses in
        # certificate subjectAltNames
        'service_identity >= 18.1.0',
        # idna 2.3 introduced some changes that break a few things.  Avoid it.
        # The problems were fixed in 2.4.
        'idna >= 0.6, != 2.3',
    ],

But I agree that it might be practicle to always have pyopenssl as a dependency.

[adiroiban]

To reduce code duplication, maybe have something like this:

    ssl = None
    skipWhenNoSSL = (True, "SSL not available")
else:
    ssl = _ssl
    skipWhenNoSSL = (False, "")

@skipIf(*skipWhenNoSSL)
def test_wantedSupported(self):

Now I know that is a lot of work, so maybe just leave the duplicate code as it is.

[adiroiban]

And since there are a lot of SSL skip conditions, maybe it would make sense to have a shared code like this

from twisted.trial import conditionals


@conditionals.skipWhenNoTLS
def test_wantedSupported(self):

In this way, all the SSL skipping code is controller from a single place.

And there can be separate conditionals for each type of extra requirements, as defined in _setup.py

[adiroiban]

Why not use the skipIf decorator here?

[adiroiban]

Thanks for the changes.

It would be nice to use this opportunity to organize the skipping conditions used in Twisted tests.

Like shared conditions for skipping on WIndows or on Unix, or when OpenSSL is missing

---

There is significant code duplication in some skip call, but we have the same duplication in trunk.

- @skipIf(not _PY35PLUS, "coroutines not available before Python 3.5")
+ onlyOnPy35Plus = not _PY35PLUS, "coroutines not available before Python 3.5"
+ @skipIf(*onlyOnPy35Plus)

---

As a drive by comment, I think that SOAP support can be removed

---

But this change is an overall improvement over trunk so I think that it can be merged.

[adiroiban]

Maybe this can be replaced by a shared skip condition for Unix... most Unix systems have pwd.
It is missing only on Win