chore: Ensure local (tox) Python testing is somewhat akin to remote (GitHub actions)

[john-bodley]

SUMMARY

It really pains me that local and remote CI testing is quite different which results in numerous wasted cycles. A case in point is for major refactors I often run tests locally (which are invoked via tox), however when running the following (as part of another PR),

 tox -e py39 -- tests/unit_tests/security/manager_test.py

the test would fail—due to a UNIQUE constraint violation—as the unit tests expect that the database schema/records are not defined, though the tox testenv environment runs the following commands prior to invoking pytest,

superset db upgrade
superset init

and thus one cannot redefine the Alpha role. Personally—per a line item I've added to the monthly Superset Meetup—I think we shouldn't differentiate between unit and integration tests and ensure that any setup works for any and all tests.

This PR simply ensures that all Python tests (be that local or remote) are invoked via the python_tests.sh script. This should work under the condition that you're running either the unit tests or integration tests, but not both.

Finally I've moved the loading of test users to leverage the load-test-users CLI command.

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a912faf) 67.17% compared to head (3b2ec96) 67.13%.
Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #27108      +/-   ##
==========================================
- Coverage   67.17%   67.13%   -0.05%     
==========================================
  Files        1900     1900              
  Lines       74430    74430              
  Branches     8293     8293              
==========================================
- Hits        50001    49969      -32     
- Misses      22374    22406      +32     
  Partials     2055     2055              

Flag	Coverage Δ
mysql	77.94% <ø> (-0.07%)	⬇️
postgres	78.04% <ø> (-0.09%)	⬇️
python	78.17% <ø> (-0.09%)	⬇️
sqlite	77.55% <ø> (-0.09%)	⬇️

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.

[michael-s-molina]

This order is different than the one specified by Integration Testing in CONTRIBUTING.md.

In the same file we have:

# Create default roles and permissions
superset init

Could you check if only the roles and permissions are created by superset init but not their associations with users? If the associations are also handled by superset init we would need to load the test users before that step.

[john-bodley]

@michael-s-molina my original intent was to follow the order you outlined in the CONTRIBUTING.md (which is referenced in a few places), however CI fails with the following,

self = <tests.integration_tests.sqllab_tests.TestSqlLab testMethod=test_query_admin_can_access_all_queries>

    def test_query_admin_can_access_all_queries(self) -> None:
        """
        Test query api with all_query_access perm added to
        Admin and make sure only Admin queries show up. This is the default
        """
        # Test search_queries for Admin user
        self.run_some_queries()
        self.login("admin")
    
        url = "/api/v1/query/"
        data = self.get_json_resp(url)
>       self.assertEqual(3, len(data["result"]))
E       AssertionError: 3 != 2

I looked into this previously, but couldn't find a resolution. I suspect this is just one of a magnitude of gremlins we have in the code base due to us not adhering to best practices. Previously the test users used to be loaded as part of the integration tests, i.e., after the superset init and I suspect there's likely some weird race condition.

[michael-s-molina]

Previously, unit tests also included ./tests/common for coverage reports.

pytest --durations-min=0.5 --cov-report= --cov=superset ./tests/common ./tests/unit_tests --cache-clear

[michael-s-molina]

There was also a --cache-clear which I don't know if it was relevant.

[michael-s-molina]

I also noticed that CI didn't run the Python-Unit / unit-tests on this PR. Could you confirm if they were skipped correctly or if they should have run?

[john-bodley]

@michael-s-molina and @mistercrunch this can probably be closed in favor of #29382 if we opt to use act rather than tox for testing locally.

[mistercrunch]

making great progress and loving act so far