Remove TXAWSTestCase

[Julian]

After #56, no more need to have a test case that's patching the environ in and out.

First time I saw this I thought there were test bugs till I figured out that the superclass was removing the hard coded list of things.

[codecov[bot]]

Codecov Report

Merging #57 into master will decrease coverage by 0.41%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #57      +/-   ##
==========================================
- Coverage   96.35%   95.93%   -0.42%     
==========================================
  Files          74       74              
  Lines        8910     8883      -27     
  Branches      628      626       -2     
==========================================
- Hits         8585     8522      -63     
- Misses        202      230      +28     
- Partials      123      131       +8

Impacted Files	Coverage Δ
txaws/testing/base.py	100% <ø> (+5.12%)	:arrow_up:
txaws/client/tests/test_ssl.py	92.46% <100%> (-0.25%)	:arrow_down:
txaws/route53/tests/test_util.py	100% <100%> (ø)	:arrow_up:
txaws/tests/test_service.py	98.61% <100%> (ø)	:arrow_up:
txaws/client/tests/test_base.py	98.28% <100%> (ø)	:arrow_up:
txaws/ec2/tests/test_client.py	99.67% <100%> (-0.01%)	:arrow_down:
txaws/s3/tests/test_client.py	100% <100%> (ø)	:arrow_up:
txaws/client/discover/tests/test_command.py	95.45% <100%> (ø)	:arrow_up:
txaws/client/ssl.py	71.25% <100%> (-3.75%)	:arrow_down:
txaws/ec2/tests/test_model.py	100% <100%> (ø)	:arrow_up:
... and 10 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6edf44c...a0024a6. Read the comment docs.

[exarkun]

Hmm. I have mixed feelings here. On one hand, yes, the freaky os.environ action-at-a-distance is rather bad. And the replacement for it, in the tests that still need to manipulate os.environ (sigh), is not too bad.

On the other hand, in other projects I've found it useful to have a base class for more or all of the tests. It's a place to decide things like which testing library to use (testtools is a strong candidate to replace trial) and to enable certain things like log collection behavior. This ends up being action-at-a-distance stuff, too, so maybe it's an anti-pattern to avoid in txaws.

What do you think?

[Julian]

I tend to pretty vehemently avoid base classes -- as a "new contributor" or even after a few months on some codebase I work on, I invariably end up staring at a subclass and needing to mentally compile what behavior the subclass introduces beyond what I already know unittest does, and I typically need to do that as like step one when writing new code to ensure that the superclass doesn't just add functionality, but where like in this case, it is going to do things that actually change the semantics of the tests I write.

I'd say my preference tends to be that if you did want one, to make sure it only added functionality (so still remove all the functionality I guess, but leave the inheritance so you can come by and add stuff, or swap out the grandparent class or whatever. TBH I don't really think that's a big deal, do u even editor regex ugh?).

But yeah even nicer to me is to not do any of that and to just use a "Well Known" baseclass whose behavior people already are forced to memorize, which lets someone at least only have to look at 2 places to fully understand the test -- the class itself, and some well known unittesting framework.

Of course, the battle for no inheritance is lost, so maybe I should give up too, so feel free to WONTFIX :/

Or yeah I'd love to hear about XYZ project where that was very useful too, I just haven't encountered one.

[exarkun]

Of course, the battle for no inheritance is lost, so maybe I should give up too, so feel free to WONTFIX :/

Yea that's the real bummer here. I know that what I'm suggesting is stupid but I feel my hands are tied.

Or yeah I'd love to hear about XYZ project where that was very useful too, I just haven't encountered one.

One useful thing I've done with this pattern in two different projects (:cry:) is fix the poor interop between testtools expectThat and Hypothesis. Without some help, Hypothesis can't see that an "expectation" failed and it doesn't really work. https://github.com/LeastAuthority/txkube/blob/master/src/txkube/testing/_testcase.py#L37 is one example of that.

I suspect there are other ways to implement that (and ideally some fix would just get pushed upstream to one or both projects and it would just work...) but it is easier if things just work if you follow the conventions.

But there's no such code that would go into the txaws base test case class right now. So it's hard to judge whether it's going to be useful or not...

[exarkun]

Okay, on further reflection, the fact that there's no current use-case for this point of centralization leads me to conclude that it's better just to wipe it. If a use-case arises in the future, then at that point in time we will be in a better position to evaluation implementation strategies.