Skip to content

test: Add support for multifile tests. #38

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

Merged
merged 1 commit into from
Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion test/helpers/expectations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ bool addToExpectations(Expectations &exp, string_view filePath, bool isDirectory
exp.minimizeRBI = filePath;
return true;
} else if (absl::EndsWith(filePath, ".rb") || absl::EndsWith(filePath, ".rbi")) {
if (!absl::EndsWith(filePath, ".snapshot.rb")) {
if (!absl::EndsWith(filePath, ".snapshot.rb") && !absl::EndsWith(filePath, "scip-ruby-args.rb")) {
exp.sourceFiles.emplace_back(filePath);
return true;
}
Expand Down Expand Up @@ -132,6 +132,7 @@ Expectations getExpectationsForFolderTest(string_view dir) {
ENFORCE(dir.back() == '/');

Expectations exp;
exp.isFolderTest = true;
// No basename; all of these files belong to this expectations.
exp.basename = "";
exp.folder = dir;
Expand All @@ -149,6 +150,7 @@ Expectations getExpectationsForTest(string_view parentDir, string_view testName)
vector<string> names = listTrimmedTestFilesInDir(parentDir, false);
bool found = false;
Expectations exp;
exp.isFolderTest = false;
exp.basename = testName.substr(parentDir.size() + 1);
exp.folder = parentDir;
exp.folder += "/";
Expand Down
1 change: 1 addition & 0 deletions test/helpers/expectations.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ struct Expectations {
// expectations type => file => expectations for file
UnorderedMap<std::string, UnorderedMap<std::string, std::string>> expectations;
std::string minimizeRBI;
bool isFolderTest;

static Expectations getExpectations(std::string singleTest);
};
Expand Down
2 changes: 1 addition & 1 deletion test/scip/BUILD
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
load(":scip_test.bzl", "scip_test_suite")

scip_test_suite(paths = glob(["testdata/*"]))
scip_test_suite(paths = glob(["testdata/*"], exclude_directories = 1), multifile_paths = glob(["testdata/multifile/*/*"]))
53 changes: 41 additions & 12 deletions test/scip/scip_test.bzl
Original file line number Diff line number Diff line change
@@ -1,20 +1,37 @@
def basename(p):
return p.rpartition("/")[-1]

def dirname(p):
return p.rpartition("/")[0]

def split_extension(p):
(before, _, ext) = basename(p).rpartition(".")
if before == "":
(before, ext) = (ext, "")
return (before, ext)

def scip_test_suite(paths):
def scip_test_suite(paths, multifile_paths):
tests = []
updates = []
for path in paths:
names = scip_test(path)
if names:
tests.append(names[0])
updates.append(names[1])

file_groups = {}
for p in multifile_paths:
d = dirname(p)
if d in file_groups:
file_groups[d].append(p)
else:
file_groups[d] = [p]

for dir, files in file_groups.items():
names = scip_multifile_test(dir, files)
tests.append(names[0])
updates.append(names[1])

native.test_suite(
name = "scip",
tests = tests,
Expand All @@ -24,22 +41,34 @@ def scip_test_suite(paths):
tests = updates,
)


def scip_test(path):
# path will end in either .snapshot, .rb or have no extension.

filename, ext = split_extension(path)
if ext != "rb":
# TODO(varun): Add support for folder tests, when there is no extension
if not path.endswith(".rb") or path.endswith(".snapshot.rb"):
return None
test_name, other_ext = split_extension(filename)
if other_ext != "":
# Don't make separate tests for .snapshot.rb files
return None

test_name = basename(path)[:-3]
snapshot_path = path[:-3] + ".snapshot.rb"
args = ["$(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)]
data = ["//test:scip_test_runner", "//test/scip:{}".format(dir)]
for filepath in filepaths:
if filepath.endswith(".rb") and not filepath.endswith(".snapshot.rb"):
# args.append("$(location {})".format(path))
data.append(filepath)
if not filepath.endswith("scip-ruby-args.rb"): # Special file for reading Gem-level args.
data.append(filepath[:-3] + ".snapshot.rb")
if not dir.startswith("testdata/multifile/"):
fail("Expected directory to be under multifile/")
dir = dir[len("testdata/multifile/"):]
if "/" in dir:
fail("Expected directory to be 1-level deep under multifile/")

test_name = "multifile_" + dir
return _make_test(test_name, args, data)

def _make_test(test_name, args, data):
native.sh_test(
name = test_name,
srcs = ["scip_test_runner.sh"],
Expand Down
10 changes: 10 additions & 0 deletions test/scip/testdata/multifile/basic/def_class1.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# typed: true

class C1
extend T::Sig

sig { returns(T::Boolean) }
def m1
true
end
end
19 changes: 19 additions & 0 deletions test/scip/testdata/multifile/basic/def_class1.snapshot.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# typed: true

class C1
# ^^ definition [..] C1#
extend T::Sig
# ^ reference [..] T#
# ^^^ reference [..] T#Sig#

sig { returns(T::Boolean) }
# ^^^ reference [..] Sorbet#Private#Static#<Class:ResolvedSig>#sig().
# ^^^^^^^ reference [..] T#Private#Methods#DeclBuilder#returns().
# ^ reference [..] T#
# ^^^^^^^ reference [..] T#Boolean.
# ^^^^^^^^^^ reference [..] Sorbet#Private#Static#ResolvedSig#
def m1
# ^^^^^^ definition [..] C1#m1().
true
end
end
6 changes: 6 additions & 0 deletions test/scip/testdata/multifile/basic/use_class1.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# typed: true
# options: showDocs

require 'def_class1'

b = C1.new.m1
13 changes: 13 additions & 0 deletions test/scip/testdata/multifile/basic/use_class1.snapshot.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# typed: true
# options: showDocs

require 'def_class1'

b = C1.new.m1
#^ definition local 2~#119448696
#documentation
#| ```ruby
#| b = T.let(_, T::Boolean)
#| ```
# ^^ reference [..] C1#
# ^^ reference [..] C1#m1().
106 changes: 78 additions & 28 deletions test/scip_test_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ namespace sorbet::test {
using namespace std;

bool update;
vector<string> inputs;
string output;
string inputFileOrDir;
string outputFileOrDir;

// Copied from pipeline_test_runner.cc
class CFGCollectorAndTyper { // TODO(varun): Copy this over to scip_test_runner.cc
Expand Down Expand Up @@ -252,19 +252,27 @@ string snapshot_path(string rb_path) {
rb_path.erase(rb_path.size() - 3, 3);
return rb_path + ".snapshot.rb";
}
struct TestSettings {
optional<string> gemMetadata;
UnorderedMap</*root-relative path*/ string, FormatOptions> formatOptions;
};

void updateSnapshots(const scip::Index &index, FormatOptions options, const std::filesystem::path &outputDir) {
void updateSnapshots(const scip::Index &index, const TestSettings &settings, const std::filesystem::path &outputDir) {
for (auto &doc : index.documents()) {
auto outputFilePath = snapshot_path(doc.relative_path());
ofstream out(outputFilePath);
if (!out.is_open()) {
FAIL(fmt::format("failed to open snapshot output file at {}", outputFilePath));
}
formatSnapshot(doc, options, out);
auto it = settings.formatOptions.find(doc.relative_path());
ENFORCE(it != settings.formatOptions.end(),
fmt::format("missing path {} as key in formatOptions map", doc.relative_path()));
formatSnapshot(doc, it->second, out);
}
}

void compareSnapshots(const scip::Index &index, FormatOptions options, const std::filesystem::path &snapshotDir) {
void compareSnapshots(const scip::Index &index, const TestSettings &settings,
const std::filesystem::path &snapshotDir) {
for (auto &doc : index.documents()) {
auto filePath = snapshot_path(doc.relative_path()); // TODO: Separate out folders!
ifstream inputStream(filePath);
Expand All @@ -275,7 +283,10 @@ void compareSnapshots(const scip::Index &index, FormatOptions options, const std
input << inputStream.rdbuf();

ostringstream out;
formatSnapshot(doc, options, out);
auto it = settings.formatOptions.find(doc.relative_path());
ENFORCE(it != settings.formatOptions.end(),
fmt::format("missing path {} as key in formatOptions map", doc.relative_path()));
formatSnapshot(doc, it->second, out);
auto result = out.str();

CHECK_EQ_DIFF(input.str(), result,
Expand Down Expand Up @@ -304,17 +315,11 @@ pair<optional<string>, FormatOptions> readMagicComments(string_view path) {
return {gemMetadata, options};
}

TEST_CASE("SCIPTest") {
// FIXME(varun): Add support for multifile tests.
ENFORCE(inputs.size() == 1);
Expectations test = Expectations::getExpectations(inputs[0]);

auto [gemMetadata, formatOptions] = readMagicComments(inputs[0]);

void test_one_gem(Expectations &test, const TestSettings &settings) {
vector<unique_ptr<core::Error>> errors;
auto inputPath = test.folder + test.basename;

auto logger = spdlog::stderr_color_mt("fixtures: " + inputPath);
auto logger =
spdlog::stderr_color_mt("fixtures: " + (test.isFolderTest ? test.folder + test.basename : test.folder));
auto errorCollector = make_shared<core::ErrorCollector>();
auto errorQueue = make_shared<core::ErrorQueue>(*logger, *logger, errorCollector);
core::GlobalState gs(errorQueue);
Expand All @@ -335,7 +340,7 @@ TEST_CASE("SCIPTest") {
using Provider = pipeline::semantic_extension::SemanticExtensionProvider;

auto indexFilePath = filesystem::temp_directory_path();
indexFilePath.append(test.basename + ".scip"); // FIXME(varun): Update for folder tests with multiple files?
indexFilePath.append(test.basename + ".scip");

auto providers = Provider::getProviders();
ENFORCE(providers.size() == 1);
Expand All @@ -344,10 +349,10 @@ TEST_CASE("SCIPTest") {
cxxopts::Options options{"scip-ruby-snapshot-test"};
scipProvider->injectOptions(options);
std::vector<const char *> argv = {"scip-ruby-snapshot-test", "--index-file", indexFilePath.c_str()};
if (gemMetadata.has_value()) {
if (settings.gemMetadata.has_value()) {
argv.push_back("--gem-metadata");
ENFORCE(!gemMetadata.value().empty());
argv.push_back(gemMetadata.value().data());
ENFORCE(!settings.gemMetadata.value().empty());
argv.push_back(settings.gemMetadata.value().data());
}
argv.push_back(nullptr);
auto parseResult = options.parse(argv.size() - 1, argv.data());
Expand Down Expand Up @@ -404,14 +409,60 @@ TEST_CASE("SCIPTest") {
index.ParseFromIstream(&indexFile);

if (update) {
updateSnapshots(index, formatOptions, test.folder);
updateSnapshots(index, settings, test.folder);
} else {
compareSnapshots(index, formatOptions, test.folder);
compareSnapshots(index, settings, test.folder);
}

MESSAGE("PASS");
}

// There are several different kinds of tests that we are potentially
// interested in here:
//
// 1. Testing single files for language features.
// 2. Testing multiple files within the same Gem to see if/when defs/refs work.
// 3. Testing usage of the --gem-metadata flag, because not all Ruby Gems
// contain a .gemspec file (e.g. if it's an application), which means
// that we do not have a reliable way of inferring the name/version.
// 4. Testing usage of Gem-internal RBI files.
// 5. Testing usage of RBI files coming from external gems. This uses
// a standardized project structure with a sorbet/ folder.
// (See https://sorbet.org/docs/rbi)
// 6. Testing usage of multiple local Gems defined within the same repo.
//
// Right now, the testing is set up to handle use cases 1, 2, 3, and 4.
// It should hopefully be straightforward to extend this support for 5.
//
// For 6, I'd like to gather more information about project setups first
// (the directory layout, any expectations of ordering, how Sorbet is run)
// before setting up testing infrastructure and explicit feature support.
TEST_CASE("SCIPTest") {
Expectations test = Expectations::getExpectations(inputFileOrDir);

TestSettings settings;
settings.gemMetadata = nullopt;
if (test.isFolderTest) {
auto argsFilePath = test.folder + "scip-ruby-args.rb";
if (FileOps::exists(argsFilePath)) {
settings.gemMetadata = readMagicComments(argsFilePath).first;
}
ENFORCE(test.sourceFiles.size() > 0);
for (auto &sourceFile : test.sourceFiles) {
auto path = test.folder + sourceFile;
auto [_, inserted] = settings.formatOptions.insert({path, readMagicComments(path).second});
ENFORCE(inserted, "duplicate source file in Expectations struct?");
}
} else {
ENFORCE(test.sourceFiles.size() == 1);
auto path = test.folder + test.sourceFiles[0];
auto &options = settings.formatOptions[path];
std::tie(settings.gemMetadata, options) = readMagicComments(path);
}

test_one_gem(test, settings);
}

} // namespace sorbet::test

// TODO: Use string.ends_with with C++20.
Expand All @@ -435,17 +486,16 @@ int main(int argc, char *argv[]) {
"path")("update-snapshots", "");
//("--update-snapshots", "should the snapshot files be overwritten if there are changes");
auto res = options.parse(argc, argv);
auto unmatched = res.unmatched();

for (auto &input : res.unmatched()) {
if (!ends_with(input, ".rb")) {
printf("error: input files must have .rb extension");
return 1;
}
if (unmatched.size() != 1) {
std::fprintf(stderr, "error: runner expected single input file with .rb extension or folder");
return 1;
}

sorbet::test::update = res.count("update-snapshots") > 0;
sorbet::test::inputs = res.unmatched();
sorbet::test::output = res["output"].as<std::string>();
sorbet::test::inputFileOrDir = unmatched[0];
sorbet::test::outputFileOrDir = res["output"].as<std::string>();

doctest::Context context(argc, argv);
return context.run();
Expand Down