chore(tests): Remove ineffectual login

[john-bodley]

SUMMARY

Whilst working on [SIP-99B] Proposal for (re)defining a "unit of work" I was running into an issue where (I thought) tests were failing due to an issue with the persistence of a logged in user whereas in actuality it was because I was missing the Public role.

The red herring was due to the slew of tests which don't interface with the test client "logging" in a user in the hope of persisting the logged in user to g.user. This is ineffectual outside of the client as the Flask globals only persist for the lifetime of the request, i.e.,

>>> from flask import g

>>> self.login(username="admin")
>>> print(g.user)
<flask_login.mixins.AnonymousUserMixin object at 0x160c48be0>

Flask-Login automatically restores g.user from that cookie if it is not in the session when dealing with requests from the client. The TL;DR is authenticating (logging in/logging out) the user is only effectual in the context of web requests via the client.

This PR removes any erroneous logins (and logouts)—determined by first eliminating all references and re-adding them for API related tests. It also cleans up any inconsistencies in how logging in occurs. I updated the login() method to remove the default admin username so now it's a required argument which aids with readability.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

CI.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[john-bodley]

Require the explicit username so it's evident who is logging in.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.44%. Comparing base (22e823b) to head (4d777bf).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #27149   +/-   ##
=======================================
  Coverage   67.43%   67.44%           
=======================================
  Files        1911     1911           
  Lines       74966    74966           
  Branches     8353     8353           
=======================================
+ Hits        50555    50558    +3     
+ Misses      22358    22355    -3     
  Partials     2053     2053           

Flag	Coverage Δ
mysql	77.92% <ø> (+<0.01%)	⬆️
postgres	78.04% <ø> (+<0.01%)	⬆️
python	78.17% <ø> (+<0.01%)	⬆️
sqlite	77.47% <ø> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[john-bodley]

@michael-s-molina I've spent/wasted hours trying to get a few failing PostgreSQL tests to pass. The tests succeed when testing locally and/or remotely when I run either i) the specific test, or ii) the specific file, however they fail when run as part of the entire test suite.

My hypothesis is that there are a slew of non-idempotent tests and this change merely (re)highlighted how fragile aspects of our tests are.

[michael-s-molina]

Yeah, it's kind of frustrating that we have many non-idempotent tests. Given that we'll need a specific effort for fixing this problem globally, I'm ok with skipping the test. We do the same for many sqlite tests.

[john-bodley]

Thanks @michael-s-molina. Also it's worth mentioning it's not like the test is being skipped globally, i.e., we have coverage on the said logic for MySQL and SQLite.