cleanup: Simplify test build & runner logic. (#151)

Author: varungandhi-src

.github/workflows/ci.yml

@@ -41,9 +41,7 @@ jobs:
         with:
           name: ci-build-log
           path: log
-      - name: "🏋️‍♂️ Run unit tests"
-        run: ./bazel test //test/scip:unit_tests --config=dbg
-      - name: "🏋️‍♂️ Run snapshot tests"
+      - name: "🏋️‍♂️ Run tests"
         run: ./bazel test //test/scip --config=dbg
       # Repo tests are kinda' broken right now because the bundle cache step needs synchronization
       #

docs/scip-ruby/CONTRIBUTING.md

@@ -154,25 +154,32 @@ There are currently 3 kinds of tests:
   to test via snapshots.
 - Repo/Integration tests: These try to index an OSS repo using scip-ruby.
 
-To run snapshot tests, which are self-contained:
+Here are some example test invocations:
 
 ```
-./bazel test //test/scip --config=dbg
-```
+# Run both snapshot tests and unit tests
+./bazel test --config=dbg //test/scip
 
-Updating snapshots:
+# Run only unit tests
+./bazel test --config=dbg //test/scip:unit_tests
 
-```
-./bazel test //test/scip:update --config=dbg
+# Run a specific snapshot test, e.g. 'testdata/alias.rb'
+./bazel test --config=dbg //test/scip:alias
 ```
 
-You can run unit tests using:
+You can add `--test_output=errors` to see diffs for snapshot mismatches.
+
+Snapshot outputs can be easily updated:
 
 ```
-./bazel test //test/scip:unit_tests --config=dbg
+# Update all snapshots
+./bazel test --config=dbg //test/scip:update
+
+# Update snapshot for a single test
+./bazel test --config=dbg //test/scip:update_alias
 ```
 
-WARNING: Repo tests are kinda' broken right now; they're disabled
+Repo tests are kinda' broken right now; they're disabled
 in CI (see ci.yml), and may or may not work on your machine.
 
 If you want to run repo tests, first complete the
@@ -258,7 +265,7 @@ to the root and run:
 ```
 
 Alternately, it may be useful to create a `tmp.rb`
-file under the `test/scip/snapshots/` directory
+file under the `test/scip/testdata/` directory
 (it will be gitignored) and run:
 
 ```bash

test/scip/BUILD

@@ -1,7 +1,7 @@
 load(":scip_test.bzl", "scip_test_suite")
 
 scip_test_suite(
-    multifile_paths = glob(["testdata/multifile/*/*"]),
+    multifile_paths = glob(["testdata/multifile/**"]),
     paths = glob(
         ["testdata/*"],
         exclude_directories = 1,

test/scip/scip_test.bzl

@@ -1,8 +1,11 @@
 def basename(p):
     return p.rpartition("/")[-1]
 
-def dirname(p):
-    return p.rpartition("/")[0]
+def ancestor_path(p, parent):
+    """Extract components of p until the second-last component is parent."""
+    before, middle, after = p.rpartition(parent + "/")
+    child, _, _ = after.partition("/")
+    return before + middle + child
 
 def split_extension(p):
     (before, _, ext) = p.rpartition(".")
@@ -11,30 +14,30 @@ def split_extension(p):
     return (before, ext)
 
 def scip_test_suite(paths, multifile_paths):
-    tests = []
+    tests = scip_unit_tests()
     updates = []
+
     for path in paths:
         names = scip_test(path)
         if names:
             tests.append(names[0])
             updates.append(names[1])
 
-    file_groups = {}
+    multifile_groups = {}
     for p in multifile_paths:
-        d = dirname(p)
-        if d in file_groups:
-            file_groups[d].append(p)
+        d = ancestor_path(p, parent = "multifile")
+        if d in multifile_groups:
+            multifile_groups[d].append(p)
         else:
-            file_groups[d] = [p]
+            multifile_groups[d] = [p]
 
-    tests.append(scip_unit_tests())
-    for dir, files in file_groups.items():
+    for dir, files in multifile_groups.items():
         names = scip_multifile_test(dir, files)
         tests.append(names[0])
         updates.append(names[1])
 
     native.test_suite(
-        name = "scip",
+        name = "scip",  # both unit tests and snapshot tests
         tests = tests,
     )
     native.test_suite(
@@ -48,23 +51,23 @@ def scip_unit_tests():
     native.sh_test(
         name = "unit_tests",
         srcs = ["scip_test_runner.sh"],
-        args = ["only_unit_tests"],
+        args = ["--only-unit-tests"],
         data = ["//test:scip_test_runner"],
         size = "small",
     )
-    return "unit_tests"
+    return ["unit_tests"]
 
 def scip_test(path):
     if not path.endswith(".rb") or path.endswith(".snapshot.rb"):
         return None
     test_name = basename(path)[:-3]
     snapshot_path = path[:-3] + ".snapshot.rb"
-    args = ["$(location {})".format(path), "--output=$(location {})".format(snapshot_path)]
+    args = ["--input=$(location {})".format(path), "--output=$(location {})".format(snapshot_path)]
     data = [path, snapshot_path, "//test:scip_test_runner"]
     return _make_test(test_name, args, data)
 
 def scip_multifile_test(dir, filepaths):
-    args = ["$(location {})".format(dir), "--output=$(location {})".format(dir)]
+    args = ["--input=$(location {})".format(dir), "--output=$(location {})".format(dir)]
     data = ["//test:scip_test_runner", "//test/scip:{}".format(dir)]
     for filepath in filepaths:
         path_without_ext, ext = split_extension(filepath)
@@ -73,10 +76,10 @@ def scip_multifile_test(dir, filepaths):
             if not filepath.endswith("scip-ruby-args.rb"):  # Special file for reading Gem-level args.
                 data.append(path_without_ext + ".snapshot." + ext)
     if not dir.startswith("testdata/multifile/"):
-        fail("Expected directory to be under multifile/")
+        fail("Expected directory to be under multifile/ but found: " + dir)
     dir = dir[len("testdata/multifile/"):]
     if "/" in dir:
-        fail("Expected directory to be 1-level deep under multifile/")
+        fail("Expected test root directory to be 1-level deep under multifile/ but found: " + dir)
 
     test_name = "multifile_" + dir
     return _make_test(test_name, args, data)

test/scip/testdata/multigem/downstream.rb

@@ -0,0 +1,7 @@
+# typed: true
+
+require 'upstream'
+
+def f()
+  g()
+end

test/scip/testdata/multigem/gem-map/gem-map.json

@@ -0,0 +1,2 @@
+{"path": "downstream.rb", "gem": "my_downstream_gem@1"}
+{"gem": "my_upstream_gem@1", "path": "upstream.rb"}

test/scip/testdata/multigem/upstream.rb

@@ -0,0 +1,5 @@
+# typed: true
+
+def g()
+  puts 'Hello'
+end

test/scip_test_runner.cc

@@ -234,8 +234,11 @@ void formatSnapshot(const scip::Document &document, FormatOptions options, std::
         return isSCIPRangeLess(occ1.range(), occ2.range());
     });
     auto formatSymbol = [](const std::string &symbol) -> string {
-        // Strip out repeating information and placeholder names.
-        return absl::StrReplaceAll(symbol, {{"scip-ruby gem ", ""}, {"placeholder_name placeholder_version", "[..]"}});
+        // Strip out repeating information for cleaner snapshots.
+        return absl::StrReplaceAll(symbol, {
+                                               {"scip-ruby gem ", ""},                           // indexer prefix
+                                               {"placeholder_name placeholder_version", "[..]"}, // test placeholder
+                                           });
     };
     size_t occ_i = 0;
     std::ifstream input(document.relative_path());
@@ -569,27 +572,31 @@ bool ends_with(const std::string &s, const std::string &suffix) {
 
 int main(int argc, char *argv[]) {
     cxxopts::Options options("test_corpus_scip", "SCIP test corpus for Sorbet typechecker");
-    options.allow_unrecognised_options().add_options()("output", "path to output file or directory",
-                                                       cxxopts::value<std::string>()->default_value(""), "path")(
-        "update-snapshots", "should the snapshot files be overwritten if there are changes");
+    options.allow_unrecognised_options();
+    options.add_options()("input", "path to input file to directory", cxxopts::value<std::string>());
+    options.add_options()("output", "path to output file or directory", cxxopts::value<std::string>());
+    options.add_options()("update-snapshots", "should the snapshot files be overwritten if there are changes");
+    options.add_options()("only-unit-tests", "only run unit tests, skip snapshot tests");
     auto res = options.parse(argc, argv);
+
     auto unmatched = res.unmatched();
+    if (!unmatched.empty()) {
+        fmt::print(stderr, "error: unexpected arguments passed to test runner {}",
+                   sorbet::scip_indexer::showVec(unmatched, [](auto &s) { return s; }));
+        return 1;
+    }
 
-    if (unmatched.size() == 1 && unmatched.front() == "only_unit_tests") {
+    if (res.count("only-unit-tests") > 0) {
         sorbet::test::onlyRunUnitTests = true;
         doctest::Context context(argc, argv);
         return context.run();
     }
 
-    if (unmatched.size() != 1) {
-        std::fprintf(stderr, "error: runner expected single input file with .rb extension or folder");
-        return 1;
-    }
-
+    ENFORCE(res.count("input") == 1);
+    ENFORCE(res.count("output") == 1);
     sorbet::test::update = res.count("update-snapshots") > 0;
-    sorbet::test::inputFileOrDir = unmatched[0];
+    sorbet::test::inputFileOrDir = res["input"].as<std::string>();
     sorbet::test::outputFileOrDir = res["output"].as<std::string>();
-
     doctest::Context context(argc, argv);
     return context.run();
 }