Vendor LoggingFixture and make fixtures, testresources hard requirements for testing#595
Conversation
|
This LGTM. ISTR the dependencies on fixtures and testresources were optional for a reason, but I can't remember what it was. Circular dependencies? @rbtcollins or @jml , do you remember? |
Circular dependencies would be a valid concern historically: both fixtures and testresources depend on testtools for their own test suites. However, as I've noted, we no longer package our tests in testtools (or in fixtures for that matter, though testresources still needs to be addressed) and packaging has evolved significantly in the past 15 years, so I don't believe this is a concern any more. |
stephenfin
force-pushed
the
fixtures-fix
branch
from
5e5b16d to
f9040f7
Compare
This was removed from fixtures recently [1]. While it has been re-added [2], the fixture is small enough that we can just vendor it here. A future change will make fixtures a hard requirement of our test suite, since that's trivially doable now that we no longer package our tests. [1] testing-cabal/fixtures#114 [2] testing-cabal/fixtures#122 Signed-off-by: Stephen Finucane <stephen@that.guru>
The tests are no longer packaged, so circular imports are less of a concern than previously. Note that this does not affect testtools itself, where both packages remain optional. Signed-off-by: Stephen Finucane <stephen@that.guru>
stephenfin
force-pushed
the
fixtures-fix
branch
from
f9040f7 to
3a23ca7
Compare
|
If we're going to add a dependency on fixtures, any reason we can't import LoggingFixture from there? |
No reason, beyond the fact that there are a few releases in the wild with the fixture missing (we need both testing-cabal/fixtures@53f7068 and testing-cabal/fixtures@f3f574a included to fix it), plus the |
2 participants
Split out from #593.
Note that we are not making these hard dependencies at runtime, only for the test suite (which is no longer packaged).