prql icon indicating copy to clipboard operation
prql copied to clipboard

Test the CLI

Open max-sixty opened this issue 3 years ago • 4 comments
trafficstars

It would be great to add tests on the CLI. These could start out very simple indeed.

max-sixty avatar Mar 12 '22 03:03 max-sixty

Branch cli-tests has a tests broken such that there is a variety of different error messages. These could be converted into CLI tests.

aljazerzen avatar Mar 29 '22 11:03 aljazerzen

We can use https://github.com/mitsuhiko/insta-cmd, just released

max-sixty avatar Jul 22 '22 02:07 max-sixty

Maybe also package the CLI as downloadable release? I don't want to use PRQL as language binding but from CLI only.

E.g. I have "supercomplex.prql" file and want to compile it to "supercomplex.sql".

This way I can use it with C# too. Secondly I commit both (SQL and PRQL) files to my repo so that I see if the queries change. It also gives me nice off-ramp if I don't want to use PRQL anymore, I just remove the file and I still have SQL file.

Ciantic avatar Aug 20 '22 11:08 Ciantic

@Ciantic Well, you already can do that via cargo install prql-compiler. There is also a homebrew recipe, but I am not sure of its stability.

aljazerzen avatar Aug 24 '22 20:08 aljazerzen

Update from the future, I was reminded about this by https://github.com/PRQL/prql/pull/1963#discussion_r1118058455:

  • We have some CLI tests, but these only test the inner methods — they don't test the full output, which is both more complete and I think a generally better test.
  • We can use https://github.com/mitsuhiko/insta-cmd to get the full test
  • This would still be a nice contribution!

max-sixty avatar Feb 26 '23 21:02 max-sixty

I tried using assert_cmd in #2488.

Like

    fn get_targets() {
        let n_targets = Target::names().len();

        assert_cmd::Command::cargo_bin("prqlc")
            .unwrap()
            .args(["get-targets"])
            .assert()
            .success()
            .stdout(
                predicates::str::is_match(r"sql\.[a-z]+\n")
                    .unwrap()
                    .count(n_targets),
            );
    }

It works well, but I removed the test because we needed to run cargo build -p prqlc before running the test.

eitsupi avatar Apr 26 '23 12:04 eitsupi

I don't get errors around being able to run the binary, but possibly I'm doing something different (sorry I missed the PR before, as I said on Discord I've been out of action for a few days, but am back now)

Here's my current diff:

diff --git a/prql-compiler/prqlc/Cargo.toml b/prql-compiler/prqlc/Cargo.toml
index df8fa177..8b9ac1f9 100644
--- a/prql-compiler/prqlc/Cargo.toml
+++ b/prql-compiler/prqlc/Cargo.toml
@@ -20,7 +20,7 @@ concolor-clap = {version = "0.1.0", features = ["api"]}
 env_logger = {version = "0.10.0", features = ["color"]}
 itertools = "0.10.3"
 notify = "^5.1.0"
-prql-compiler = {path = '..', version = "0.8.0" }
+prql-compiler = {path = '..', version = "0.8.0"}
 regex = {version = "1.8.1", features = ["std", "unicode"]}
 serde = "^1"
 serde_json = "1.0.81"
@@ -32,4 +32,5 @@ walkdir = "^2.3.2"
 minijinja = {version = "=0.31.0", features = ["unstable_machinery"]}
 
 [target.'cfg(not(target_family="wasm"))'.dev-dependencies]
+assert_cmd = "2.0.11"
 insta = {version = "1.29", features = ["colors", "glob", "yaml"]}
diff --git a/prql-compiler/prqlc/tests/test.rs b/prql-compiler/prqlc/tests/test.rs
new file mode 100644
index 00000000..c36febf3
--- /dev/null
+++ b/prql-compiler/prqlc/tests/test.rs
@@ -0,0 +1,20 @@
+#![cfg(not(target_family = "wasm"))]
+
+use prql_compiler::Target;
+
+#[test]
+fn get_targets() {
+    use assert_cmd;
+    let n_targets = Target::names().len();
+
+    assert_cmd::Command::cargo_bin("prqlc")
+        .unwrap()
+        .args(["get-targets"])
+        .assert()
+        .success()
+        .stdout(
+            "hello", // predicates::str::is_match(r"sql\.[a-z]+\n")
+                    //     .unwrap()
+                    //     .count(n_targets),
+        );
+}

I do get errors on predicates — is that a library?


Alternatively we could use https://github.com/mitsuhiko/insta-cmd — this is nice and consistent with our existing insta tests.


If we have a test that really can't run on cargo test, we can use a feature to opt-in, and run that feature after setup in GHA. We don that with integration tests

max-sixty avatar Apr 27 '23 07:04 max-sixty

I do get errors on predicates — is that a library?

Yes, this is another crate recommended on the assert_cmd document (removed via fd0984c10f1efa423f6132a0992f0119df1c501d).

assert_cmd seems to be maintained by a clap maintainer and is often used with clap.

If we have a test that really can't run on cargo test, we can use a feature to opt-in, and run that feature after setup in GHA. We don that with integration tests

It would be wonderful!

eitsupi avatar Apr 27 '23 13:04 eitsupi