-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add Metabase tests #20
Merged
Merged
Changes from 40 commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
db1398c
Add Metabase tests
bobbyiliev bdf6d9f
Add dpes.edn file
bobbyiliev 4bb49f4
Update the dpes.edn file
bobbyiliev 7f45b5b
Add default test datasets
bobbyiliev 8e8e088
dislabe ssl for test container
bobbyiliev 999d214
Try to disable set-timezone
bobbyiliev 65abc00
Add init container to increase max_tables
bobbyiliev feb2aba
Disable setting timezone
bobbyiliev 81eabed
Try to fix expression-using-aggregation-test
bobbyiliev 377406f
Fixing tests
bobbyiliev 221afa2
Disable percentile-aggregations due to missing func
bobbyiliev 5834e27
Add json type
bobbyiliev c64e15c
Fix weeks interval tests
bobbyiliev a1dbc6f
Fix weeks interval tests
bobbyiliev 99f784e
Upgrade node and clojure versions
bobbyiliev cd06813
Upgrade Metabase version
bobbyiliev f210deb
Change jdk from 17 to 11
bobbyiliev 4c7ba86
Exclude materialize from faulty timezone tests
bobbyiliev 79acf7a
Disable :temporal-extract
bobbyiliev 94f36be
Re enable :temporal-extract
bobbyiliev de8d99d
Re enable :temporal-extract
bobbyiliev 1a5471a
Disable :temporal-extract during tests
bobbyiliev 625a86a
Disable test with failing FK
bobbyiliev a68ef44
Disable test with failing FK
bobbyiliev 68f1f20
Fix failing FK tests
bobbyiliev fc1a80f
Disable test due to inconsistent results
bobbyiliev 0e79404
Fix git diff apply
bobbyiliev cd56201
Add cluster conn option
bobbyiliev 3fbbcaf
Add CONTRIBUTING.md file
bobbyiliev f1d7695
Update readme and add build script
bobbyiliev 2dd304e
Update resources/metabase-plugin.yaml
bobbyiliev 5d3a884
Small fixes
bobbyiliev 5140f9e
Update contributing.md file
bobbyiliev 91f2919
Small fixes
bobbyiliev 9193399
PR change requests
bobbyiliev 1922569
Sanitize the conn details
bobbyiliev 9aab1bb
Update cluster display name
bobbyiliev 7f26855
Update src/metabase/driver/materialize.clj
bobbyiliev dee4f9b
Add hostname and port validation
bobbyiliev dc9d4a6
Remove port validation
bobbyiliev 51c7c2e
Add release version table
bobbyiliev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{:aliases | ||
{:user/materialize | ||
{:extra-paths ["PWD/modules/drivers/materialize/test"] | ||
:extra-deps {metabase/materialize {:local/root "PWD/modules/drivers/materialize"}}}}} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
name: Tests | ||
|
||
on: | ||
workflow_dispatch: | ||
push: | ||
branches: | ||
- master | ||
paths-ignore: | ||
- "**.md" | ||
pull_request: | ||
|
||
jobs: | ||
test: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Checkout Metabase Repo | ||
uses: actions/checkout@v3 | ||
with: | ||
repository: metabase/metabase | ||
ref: v0.46.7 | ||
|
||
- name: Checkout Driver Repo | ||
uses: actions/checkout@v3 | ||
with: | ||
path: modules/drivers/materialize | ||
|
||
- name: Set up JDK 11 | ||
uses: actions/setup-java@v2 | ||
with: | ||
distribution: temurin | ||
java-version: 11 | ||
|
||
- name: Add Materialize TLS instance to /etc/hosts | ||
run: | | ||
sudo echo "127.0.0.1 materialize" | sudo tee -a /etc/hosts | ||
|
||
- name: Start Materialize in Docker | ||
uses: isbang/[email protected] | ||
with: | ||
compose-file: "modules/drivers/materialize/docker-compose.yml" | ||
services: | | ||
materialize | ||
init | ||
|
||
# Apply the scripts/exclude_tests.diff patch to exclude tests that are not relevant to Materialize | ||
- name: Apply exclude_tests.diff | ||
run: | | ||
git apply modules/drivers/materialize/scripts/exclude_tests.diff | ||
|
||
- name: Install Clojure CLI | ||
run: | | ||
curl -O https://download.clojure.org/install/linux-install-1.11.1.1262.sh && | ||
sudo bash ./linux-install-1.11.1.1262.sh | ||
|
||
- name: Setup Node | ||
uses: actions/setup-node@v2 | ||
with: | ||
node-version: "16" | ||
cache: "yarn" | ||
|
||
- name: Get M2 cache | ||
uses: actions/cache@v2 | ||
with: | ||
path: | | ||
~/.m2 | ||
~/.gitlibs | ||
key: ${{ runner.os }}-materialize-${{ hashFiles('**/deps.edn') }} | ||
|
||
- name: Prepare stuff for pulses | ||
run: yarn build-static-viz | ||
|
||
# Use custom deps.edn containing "user/materialize" alias to include driver sources | ||
- name: Run tests | ||
run: | | ||
mkdir -p /home/runner/.config/clojure | ||
cat modules/drivers/materialize/.github/deps.edn | sed -e "s|PWD|$PWD|g" > /home/runner/.config/clojure/deps.edn | ||
DRIVERS=materialize clojure -X:dev:drivers:drivers-dev:test:user/materialize | ||
|
||
- name: Build Materialize driver | ||
run: | | ||
echo "{:deps {metabase/materialize {:local/root \"materialize\" }}}" > modules/drivers/deps.edn | ||
bin/build-driver.sh materialize | ||
ls -lah resources/modules | ||
|
||
- name: Archive driver JAR | ||
uses: actions/upload-artifact@v3 | ||
with: | ||
name: materialize.metabase-driver.jar | ||
path: resources/modules/materialize.metabase-driver.jar |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
/target | ||
.clj-kondo/ | ||
.lsp/ | ||
.cpcache/ | ||
.build |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
## Getting started | ||
|
||
* Please report any issues you encounter during operations. | ||
* Feel free to create a pull request, preferably with a test or five. | ||
|
||
## Setting up a development environment | ||
|
||
### Requirements | ||
|
||
* Clojure 1.11+ | ||
* OpenJDK 17 | ||
* Node.js 16.x | ||
* Yarn | ||
|
||
For testing: Docker Compose | ||
|
||
Please refer to the extensive documentation available on the Metabase website: [Guide to writing a Metabase driver](https://www.metabase.com/docs/latest/developers-guide/drivers/start.html) | ||
|
||
Materialize driver's code should be inside the main Metabase repository checkout in `modules/drivers/materialize` directory. | ||
|
||
The easiest way to set up a development environment is as follows (mostly the same as in the [CI](https://github.com/MaterializeInc/metabase-materialize-driver/blob/master/.github/workflows/tests.yml)): | ||
|
||
* Clone Metabase and Materialize driver repositories | ||
```bash | ||
git clone https://github.com/metabase/metabase.git | ||
cd metabase | ||
checkout v0.46.7 | ||
git clone https://github.com/MaterializeInc/metabase-materialize-driver.git modules/drivers/materialize | ||
``` | ||
|
||
* Create custom Clojure profiles, you can get it using the following command: | ||
|
||
```bash | ||
cat modules/drivers/materialize/.github/deps.edn | sed -e "s|PWD|$PWD|g" | tr -d '\n' | ||
``` | ||
|
||
Modifying `~/.clojure/deps.edn` will create two useful profiles: `user/materialize` that adds driver's sources to the classpath, and `user/test` that includes all the Metabase tests that are guaranteed to work with the driver. | ||
|
||
* Install the Metabase dependencies: | ||
|
||
```bash | ||
clojure -X:deps:drivers prep | ||
``` | ||
|
||
* Build the frontend: | ||
|
||
```bash | ||
yarn && yarn build-static-viz | ||
``` | ||
|
||
* Add `/etc/hosts` entry | ||
|
||
Required for TLS tests. | ||
|
||
```bash | ||
sudo -- sh -c "echo 127.0.0.1 materialize >> /etc/hosts" | ||
``` | ||
|
||
* Start Materialize as a Docker container | ||
|
||
```bash | ||
docker compose -f modules/drivers/materialize/docker-compose.yml up -d materialize init | ||
``` | ||
|
||
Now, you should be able to run the tests: | ||
|
||
```bash | ||
mz_deps=$(cat modules/drivers/materialize/.github/deps.edn | sed -e "s|PWD|$PWD|g" | tr -d '\n') | ||
DRIVERS=materialize clojure -Sdeps ${mz_deps} -X:dev:drivers:drivers-dev:test:user/materialize | ||
``` | ||
|
||
you can see that we have our profiles `:user/materialize:user/test` added to the command above, and with `DRIVERS=materialize` we instruct Metabase to run the tests only for Materialize. | ||
|
||
> **Note** Omitting `DRIVERS` will run the tests for all the built-in database drivers. | ||
|
||
If you want to run tests for only a specific test: | ||
|
||
```bash | ||
mz_deps=$(cat modules/drivers/materialize/.github/deps.edn | sed -e "s|PWD|$PWD|g" | tr -d '\n') | ||
DRIVERS=materialize clojure -Sdeps ${mz_deps} -X:dev:drivers:drivers-dev:test:user/materialize :only metabase.query-processor.middleware.parameters.mbql-test | ||
``` | ||
|
||
## Building a jar | ||
|
||
You need to add an entry for Materialize in `modules/drivers/deps.edn` | ||
|
||
```clj | ||
{:deps | ||
{... | ||
metabase/materialize {:local/root "materialize"} | ||
...}} | ||
``` | ||
|
||
or just run this from the root Metabase directory, overwriting the entire file: | ||
|
||
```bash | ||
echo "{:deps {metabase/materialize {:local/root \"materialize\" }}}" > modules/drivers/deps.edn | ||
``` | ||
|
||
Now, you should be able to build the final jar: | ||
|
||
```bash | ||
bin/build-driver.sh materialize | ||
``` | ||
|
||
As the result, `resources/modules/materialize.metabase-driver.jar` should be created. | ||
|
||
For smoke testing, there is a Metabase with the link to the driver available as a Docker container: | ||
|
||
```bash | ||
docker compose -f modules/drivers/materialize/docker-compose.yml up -d metabase | ||
``` | ||
|
||
It should pick up the driver jar as a volume. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
ARG METABASE_VERSION=latest | ||
FROM metabase/metabase:${METABASE_VERSION} | ||
|
||
ADD target/dist/materialize-driver.jar /plugins/ | ||
ADD .build/materialize-driver.jar /plugins/ | ||
RUN chmod 744 /plugins/materialize-driver.jar |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test takes 12 minutes because it tests all the drivers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it only tests Materialize as we add the
DRIVERS=materialize
before the clojure command. It's just that there are too many tests, around 15k assertions.