Mapping tests: restructure, refactor, decouple session and engine, improve

[jdavcs]

Summary of changes:

1. Do not use Galaxy's connection engine and session for testing mapping.
   The tests reused the session object defined in galaxy.model (which used an engine built for a functioning Galaxy). With that setup, in addition to testing the mapping, we also unintentionally tested Galaxy's session and engine setup (thus, "testing by coincidence"). Here we create a separate engine and session, ensuring that their setup does not interfere with the test. (see b0c291c)
2. Restructure files; factor out common fixtures into conftest.py (which is why we need a separate directory) and common code into common.py. Address technical dept created in Convert install_model to declarative mapping #12756. (duplicate code in TS mapping tests still needs to be addressed; depends on how much we can/should isolate TS code base)
3. Apply changes from Verify post-test cleanup for model mapping unit tests; fix a few errors #12714. (reviewed/approved)
4. Improve test helper w.r.t. comparing collections of mapped objects using their primary keys to determine equality; add tests exposing missed cases. Justification in description of 43906e0. (Also see 518cc51, a834b54)

Note: This was supposed to get rid of code, not add! However, I used black with default settings on test_model_mapping.py to get rid of all the long lines that were a result of replacing a simple equality with a call to the new helper function. I don't love the formatting, but I love not having an opinion on how to format 7K lines 😄
(I don't mind reformatting it back if needed)

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.

License

• I agree to license these contributions under Galaxy's current license.
• I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to [email protected].

[jmchilton]

I feel like mapping.init("/tmp", db_uri, create_tables=True) is our interface to the mapping so I don't think I would skip that code for the unit tests. I would pragmatically chose to test what we use how we use it over a more pure unit test, but this may just a reflect an ignorance about the individual components on my part.

[jdavcs]

I feel like mapping.init("/tmp", db_uri, create_tables=True) is our interface to the mapping so I don't think I would skip that code for the unit tests. I would pragmatically chose to test what we use how we use it over a more pure unit test...

@jmchilton Thanks!
I deliberated over this. It was convenient: the mapping tests triggered our session and engine setup, so additional relevant warnings were reported. But I couldn't isolate them; as a result it was unclear whether a particular SQLAlchemy warning was triggered by our session setup (autocommit=True, binding SA's MetaData to a Connectable, and something else, I think), OR incorrect mapping - which is what the tests were meant to expose. Decoupling the session+engine setup from the mapping tests solved this issue.

That said, I will be adding additional unit tests covering our session and engine setup. I need that for fixing the SQLAlchemy removed-in-2.0 warnings; and also for modifying some of that code for Alembic.