Skip to content

Commit

Permalink
Update test runner to support more than 2 typechecking paths (sorbet#…
Browse files Browse the repository at this point in the history
…6453)

* Make `canTakeFastPath` an emum with 2 options instead of a bool

Also do a renaming

`canTakeFastPath` -> `getTypecheckingPath`

* Update test runner to support more than 2 typechecking paths
  • Loading branch information
ilyailya authored Oct 6, 2022
1 parent bc72f90 commit f919e12
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 16 deletions.
20 changes: 15 additions & 5 deletions main/lsp/LSPTypechecker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,21 @@
namespace sorbet::realmain::lsp {
using namespace std;
namespace {

TypecheckingPath toTypecheckingPath(PathType pathType) {
switch (pathType) {
case PathType::Slow:
return TypecheckingPath::Slow;
case PathType::Fast:
return TypecheckingPath::Fast;
}
}

void sendTypecheckInfo(const LSPConfiguration &config, const core::GlobalState &gs, SorbetTypecheckRunStatus status,
bool isFastPath, std::vector<core::FileRef> filesTypechecked) {
PathType typecheckingPath, std::vector<core::FileRef> filesTypechecked) {
if (config.getClientConfig().enableTypecheckInfo) {
auto sorbetTypecheckInfo =
make_unique<SorbetTypecheckRunInfo>(status, isFastPath, config.frefsToPaths(gs, filesTypechecked));
auto sorbetTypecheckInfo = make_unique<SorbetTypecheckRunInfo>(status, toTypecheckingPath(typecheckingPath),
config.frefsToPaths(gs, filesTypechecked));
config.output->write(make_unique<LSPMessage>(
make_unique<NotificationMessage>("2.0", LSPMethod::SorbetTypecheckRunInfo, move(sorbetTypecheckInfo))));
}
Expand Down Expand Up @@ -174,7 +184,7 @@ bool LSPTypechecker::typecheck(LSPFileUpdates updates, WorkerPool &workers,
vector<core::FileRef> filesTypechecked;
bool committed = true;
const bool isFastPath = updates.typecheckingPath == PathType::Fast;
sendTypecheckInfo(*config, *gs, SorbetTypecheckRunStatus::Started, isFastPath, {});
sendTypecheckInfo(*config, *gs, SorbetTypecheckRunStatus::Started, updates.typecheckingPath, {});
{
ErrorEpoch epoch(*errorReporter, updates.epoch, isFastPath, move(diagnosticLatencyTimers));

Expand All @@ -190,7 +200,7 @@ bool LSPTypechecker::typecheck(LSPFileUpdates updates, WorkerPool &workers,
}

sendTypecheckInfo(*config, *gs, committed ? SorbetTypecheckRunStatus::Ended : SorbetTypecheckRunStatus::Cancelled,
isFastPath, move(filesTypechecked));
updates.typecheckingPath, move(filesTypechecked));
return committed;
}

Expand Down
4 changes: 3 additions & 1 deletion main/lsp/tools/make_lsp_types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1361,10 +1361,12 @@ void makeLSPTypes(vector<shared_ptr<JSONClassType>> &enumTypes, vector<shared_pt
auto SorbetTypecheckRunStatus =
makeIntEnum("SorbetTypecheckRunStatus", {{"Started", 0}, {"Cancelled", 1}, {"Ended", 2}}, enumTypes);

auto TypecheckingPath = makeStrEnum("TypecheckingPath", {"fast", "slow"}, enumTypes);

auto SorbetTypecheckRunInfo = makeObject("SorbetTypecheckRunInfo",
{
makeField("status", SorbetTypecheckRunStatus),
makeField("fastPath", JSONBool),
makeField("typecheckingPath", TypecheckingPath),
makeField("filesTypechecked", makeArray(JSONString)),
},
classTypes);
Expand Down
2 changes: 1 addition & 1 deletion test/helpers/position_assertions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1121,7 +1121,7 @@ FastPathAssertion::FastPathAssertion(string_view filename, unique_ptr<Range> &ra
void FastPathAssertion::check(SorbetTypecheckRunInfo &info, string_view folder, int updateVersion,
string_view errorPrefix) {
string updateFile = fmt::format("{}.{}.rbupdate", filename.substr(0, -3), updateVersion);
if (!info.fastPath) {
if (info.typecheckingPath != TypecheckingPath::Fast) {
ADD_FAIL_CHECK_AT(updateFile.c_str(), assertionLine,
errorPrefix << "Expected file update to take fast path, but it took the slow path.");
}
Expand Down
27 changes: 18 additions & 9 deletions test/lsp_test_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ TEST_CASE("LSPTest") {
INFO(errorPrefix
<< "For stale state tests, we always expect Sorbet to take slow path, "
"but it took the fast path.");
CHECK_EQ(params->fastPath, false);
CHECK_NE(params->typecheckingPath, TypecheckingPath::Fast);
});

// Check any new HoverAssertions in the updates.
Expand All @@ -937,16 +937,25 @@ TEST_CASE("LSPTest") {
// in this codepath since we are running in single-threaded mode.
verifyTypecheckRunInfo(
errorPrefix, responses, SorbetTypecheckRunStatus::Ended, ExpectDiagnosticMessages::Yes,
[&errorPrefix, assertSlowPath, &assertFastPath, &test, &version](auto &params) -> void {
if (assertSlowPath.has_value()) {
if (params->fastPath && assertSlowPath.value()) {
INFO(errorPrefix << "Expected Sorbet to take slow path, but it took the fast path.");
CHECK_NE(params->fastPath, assertSlowPath.value());
} else if (!params->fastPath && !assertSlowPath.value()) {
[&errorPrefix, assertSlowPath, &assertFastPath, &test,
&version](unique_ptr<SorbetTypecheckRunInfo> &params) -> void {
auto validateAssertions = [&params, &errorPrefix](TypecheckingPath path, string actualPath,
bool assertValue, string expectedPath) {
auto isSelectedPath = params->typecheckingPath == path;
if (isSelectedPath && assertValue) {
INFO(errorPrefix
<< fmt::format("Expected Sorbet to take {} path, but it took the {} path.",
expectedPath, actualPath));
CHECK_NE(isSelectedPath, assertValue);
} else if (!isSelectedPath && !assertValue) {
INFO(errorPrefix
<< "Expected Sorbet to not take slow path, but it took the slow path.");
CHECK_NE(params->fastPath, assertSlowPath.value());
<< fmt::format("Expected Sorbet to take {} path, but it took the {} path.",
expectedPath, actualPath));
CHECK_NE(isSelectedPath, assertValue);
}
};
if (assertSlowPath.has_value()) {
validateAssertions(TypecheckingPath::Fast, "fast", assertSlowPath.value(), "slow");
}
if (assertFastPath.has_value()) {
(*assertFastPath)->check(*params, test.folder, version, errorPrefix);
Expand Down

0 comments on commit f919e12

Please sign in to comment.