feat: get docker-compose to work as the backend for Cypress tests #31796
Conversation
Running Cypress tests as depicted in docs is difficult and error-prone. Some of the challenges include: - having a reproducible deterministic setup (version of python that matches CI) - switching to a different `superset_config` - requires a local postgres - database state from previous branch can interfere in and out of a cypress setup (different set of examples, database migrations might be in a different checkpoint, ...) - switching to a different port - having celery workers working - caching is nice to have to accelerate tests, which may or may not be set up - ... Given this, I thought I'd just make it such that you can just `CYPRESS_CONFIG=true docker compose up` to fire up a backend against which you can run cypress tests locally. Given the reference to `CYPRESS_CONFIG` in our docker-compose.yml and in various `docker/`-related scripts, it looks like either it was supported in the past, or someone tried to do work in this direction, but clearly isn't working today as is. To get this working here I'm: - setting up `CYPRESS_CONFIG=true` to point to a dedicated database schema `superset_cypress`, this gets created on Postgres first-boot, and currently will require deleting the docker volume for Postgres to take effect - starting up superset app to run on `8081`, which seems to be the default port assumption for Cypress runs - pointing to the config file at `tests/integration_tests/superset_test_config.py`, which is need for tests, eventually might want to merge this into `docker/pythonpath/superset_config.py` for simplicity. For now I conditionally import it - removing duplicate docs entry existing in both `docs/docs/contributing/development.mdx` and `docs/docs/contributing/howtos.mdx` - adjusting docs As of current commit, tests don't seem to pass, so there's probably some things I'm missing, have to dig deeper
|
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment
|
| @@ -89,6 +89,8 @@ services: | |||
| restart: unless-stopped | |||
| ports: | |||
| - 8088:8088 | |||
| # When in cypress-mode -> | |||
| - 8081:8081 | |||
There was a problem hiding this comment.
NOTE: debated whether to try and open this port conditionally, but seemed impossible and researched whether there's downside in opening a port-to-nowhere in other scenarios and I seemed there's no downside. Could also just run Cypress against 8088 too if we prefer
| set -e | ||
|
|
||
| psql -v ON_ERROR_STOP=1 --username "${POSTGRES_USER}" <<-EOSQL | ||
| CREATE DATABASE superset_cypress; |
There was a problem hiding this comment.
NOTE: this script runs only on first-init of the postgres image, so will require devs to either delete their postgres-supporting volume (one easy command) or to shell into psql and CREATE DATABASE. Considering adding a NUKE_DATABASE flag more broadly in docker-compose too
| # located @ tests/integration_tests/superset_test_config.py | ||
| base_dir = os.path.dirname(__file__) | ||
| module_folder = os.path.abspath( | ||
| os.path.join(base_dir, "../../tests/integration_tests/") |
There was a problem hiding this comment.
NOTE: conditionally loading config located at tests/integration_tests/, but would much rather deprecate that file and merge non-conflicting/duplicated configs directly in here. Doing this would need adjustments on CI scripts and could break local envs (probably ok as we would instruct devs to work in docker compose)
| @@ -631,71 +631,6 @@ To run a single test file: | |||
| npm run test -- path/to/file.js | |||
| ``` | |||
|
|
|||
| ### Integration Testing | |||
There was a problem hiding this comment.
NOTE: these docs, and many other entries in these files are duplicated in ./howtos.mdx . It's a bit tricky as they are not pure dups as there has been edits on both sides
| @@ -1923,7 +1923,7 @@ class ExtraDynamicQueryFilters(TypedDict, total=False): | |||
| "Failed to import config for %s=%s", CONFIG_PATH_ENV_VAR, cfg_path | |||
| ) | |||
| raise | |||
| elif importlib.util.find_spec("superset_config") and not is_test(): | |||
There was a problem hiding this comment.
NOTE: this seemed to say "if you're in test mode, DONT load ANY custom config", which is a confusing hack and probably doesn't belong here or anywhere
| @@ -26,6 +26,10 @@ require('cy-verify-downloads').addCustomCommand(); | |||
|
|
|||
| // fail on console error, allow config to override individual tests | |||
| // these exceptions are a little pile of tech debt | |||
| // | |||
|
|
|||
| // DISABLING FOR NOW | |||
There was a problem hiding this comment.
Ok so now I know no one has used cypress locally in a while since any console error/warning including React ones fails the tests. From my understanding the reason why it works in CI is because the npm run build suppresses these console errors.
I'm thinking we disable for now and re-enable later after cleaning up all React warnings, which is beyond the scope of this PR
| @@ -27,7 +27,8 @@ export default eyesPlugin( | |||
| chromeWebSecurity: false, | |||
| defaultCommandTimeout: 8000, | |||
| numTestsKeptInMemory: 0, | |||
| experimentalFetchPolyfill: true, | |||
| // Disabled after realizing this MESSES UP rison encoding in intricate ways | |||
| experimentalFetchPolyfill: false, | |||
There was a problem hiding this comment.
This bug was gnarly to debug. Screw this experimental feature.
sadpandajoe
added
the
review:checkpoint
Running Cypress tests as depicted in docs is difficult and error-prone. Some of the challenges include:
superset_configGiven this, I thought I'd just make it such that you can just
CYPRESS_CONFIG=true docker compose upto fire up a backend against which you can run cypress tests locally. Given the reference toCYPRESS_CONFIGin our docker-compose.yml and in variousdocker/-related scripts, it looks like either it was supported in the past, or someone tried to do work in this direction, but clearly isn't working today as is.To get this working here I'm:
CYPRESS_CONFIG=trueto point to a dedicated database schemasuperset_cypress, this gets created on Postgres first-boot, and currently will require deleting the docker volume for Postgres to take effect8081, which seems to be the default port assumption for Cypress runstests/integration_tests/superset_test_config.py, which is need for tests, eventually might want to merge this intodocker/pythonpath/superset_config.pyfor simplicity. For now I conditionally import itdocs/docs/contributing/development.mdxanddocs/docs/contributing/howtos.mdxaddressing current issues
I won't lie getting this back on track feels like wacking a machete through the jungle. Borderline disrepair territory. Turns out probably no one has been running tests locally for a while cause the setup was broken af.