Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweaks for integration tests #2353

Closed
1 of 7 tasks
Jelenkee opened this issue Mar 30, 2023 · 1 comment
Closed
1 of 7 tasks

Tweaks for integration tests #2353

Jelenkee opened this issue Mar 30, 2023 · 1 comment
Labels

Comments

@Jelenkee
Copy link
Contributor

Jelenkee commented Mar 30, 2023

What's up?

Some follow-ups for #2286

  • (minor) Possibly output the snapshots as CSVs / something both machine & person-readable, e.g. with headers, rather than one long line
  • (minor) Consider whether we keep it in the current CI test vs move to another, possibly in test-all.yaml. It currently adds 40 seconds, which is not too bad. Or we condition the test-rust tests on a PR's paths, and avoid running it for small doc changes.
  • When starting docker compose the owner of prql-compiler/tests/integration/data/chinook is changed on Linux. Probably on macOS too. That should be fixed because you cannot edit the files in there. chore(test): bind mount read only in compose file for testing #2352

(edited by @max-sixty to add an item that @Jelenkee suggested. And now completed)

@max-sixty
Copy link
Member

Implement whether to run external integration tests with a feature, rather than reading the CI env var

Specifically for this, I was thinking:

  • We have a feature in prql-complier called something like test-external-dbs, which is not default
  • We put the list of tests to run under the feature (or use cfg-if if needed), so it's the longer list when that feature is active
  • Run in CI with --all-features
  • Remove the special-case code around CI from the library
  • (minor) Also put dependencies behind this feature, so we didn't need to compile them otherwise

This would mean we get dependable, hermetic external tests, and still get the in-process tests by default

max-sixty added a commit that referenced this issue Apr 3, 2023
#2353

---------

Co-authored-by: Maximilian Roos <5635139+max-sixty@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: eitsupi <50911393+eitsupi@users.noreply.github.com>
@Jelenkee Jelenkee closed this as completed Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants