Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a first-class testing framework for FadeBasic: a new test ... endtest language construct with assert, runto, and mock keywords, supporting opcodes (RUNTO, ASSERT_FAIL, MOCK_*, SPREAD/GATHER_ARRAY, LENGTH, etc.), an ITestLaunchable extensibility point, NuGet packages (FadeBasic.Testing, FadeBasic.TestAdapter) that integrate with dotnet test via Microsoft.Testing.Platform and VSTest, and a large suite of new tests. Editor extensions (VS Code, Rider) are also touched for DAP/LSP wiring.
Changes:
- New testing language:
test/endtestblocks,runto,assert,mock,call count,lenkeyword/opcode, with corresponding lexer, parser, compiler, AST, and VM changes - New runtime/SDK surfaces:
FadeRuntimeContext.RunTest/RunAllTests,Launcher.Main<T>/TryDispatchTestArgs,LaunchUtil.PackTestManifest, generatedpartiallaunchable implementingITestLaunchable - New
FadeBasic.Testing+FadeBasic.TestAdapterpackages enablingdotnet testdiscovery/execution; VS Code DAP gainsdotnetCommand: "run"|"test"; Rider Gradle build supports-PfadeJdkVersion
Reviewed changes
Copilot reviewed 97 out of 98 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| VsCode/basicscript/src/extension.ts | Adds FADE_BASIC_DEBUG_DOTNET_COMMAND env; activates a hardcoded developer-machine LSP config (critical) |
| VsCode/basicscript/package.json | Adds dotnetCommand enum to debug schema |
| Rider/fade-basic-rider/README.md | Hardcoded macOS JBR path replaces the generic JDK placeholder |
| Rider/fade-basic-rider/build.gradle.kts | Makes JDK/jvmTarget overridable via -PfadeJdkVersion |
| FadeBasic/Tests/Tests.csproj | Bumps Microsoft.NET.Test.Sdk to 18.0.1 and references new testing/adapter projects |
| FadeBasic/Tests/TestManifestPackingTests.cs | New round-trip tests for LaunchUtil.PackTestManifest and synthetic launchable dispatch |
| FadeBasic/Tests/TestFunctionTests.cs | New tests covering test-scoped functions/labels; includes a tautological assertion |
| FadeBasic/Tests/TestExecutionTests.cs | New end-to-end test-execution tests; one test (Execute_GlobalVariables) lacks assertions |
| FadeBasic/Tests/TokenVm.cs | Renames test-only identifiers (test→demo) to avoid clashing with the new keyword |
| FadeBasic/Tests/TokenVm_GC.cs | Same rename for GC tests |
| FadeBasic/Tests/TokenMacroTests.cs | Same rename for macro tests |
| FadeBasic/Tests/MusicFbasicReproTests.cs | New repro test; uses runto lbl2: with a suspicious trailing colon |
| FadeBasic/Tests/LenKeywordTests.cs | New tests for len keyword; empty-string case can't actually verify the result |
| FadeBasic/Tests/ParserTests_Erros.cs | Adds a parser-error test with off indentation |
| FadeBasic/FadeBasic.sln, build.sln | Stray # line inserted into the solution header |
| FadeBasic/FadeBasic.Lib.Standard/Commands/Standard/StringCommands.cs | Removes len host command (now a language keyword) — breaking |
| FadeBasic/FadeBasic/Virtual/OpCodes.cs | Adds many new opcodes; renumbers LENGTH (bytecode-breaking) |
Comments suppressed due to low confidence (1)
VsCode/basicscript/src/extension.ts:176
- This block hardcodes a developer-machine-specific absolute path (
/Users/chrishanna/Documents/Github/dby/FadeBasic/LSP) and unconditionally overrides the previously-setconfig(the stdio-transport configuration assigned just above). Shipping this would break the extension for every other user — the LSP would attempt to spawn from a path that only exists on one machine. This block was previously commented out for the same reason; it should be re-commented (or removed) before merging, leaving only the productiontransport: TransportKind.stdioconfig active.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ```bash | ||
| export JAVA_HOME="<path-to-jdk-17+>" | ||
| export JAVA_HOME="/Applications/IntelliJ IDEA.app/Contents/jbr/Contents/Home" |
| @@ -383,8 +382,181 @@ public static class OpCodes | |||
|
|
|||
| /// <summary> | |||
| /// pull a value off the defer stack, and push it into the main stack. | |||
| /// If the defer stack is empty, a 0 is used. | |||
| /// If the defer stack is empty, a 0 is used. | |||
| /// </summary> | |||
| public const byte POP_DEFER = 63; | |||
|
|
|||
| /// <summary> | |||
| /// Begin a runto: pops a target address off the stack, pushes a runto frame | |||
| /// (target_addr, test_resume_ip), and sets instructionIndex to the saved | |||
| /// programResumeIP. Used at the start of every `runto :L` statement in test code. | |||
| /// </summary> | |||
| public const byte RUNTO = 64; | |||
|
|
|||
| /// <summary> | |||
| /// Yield-back marker: the compiler emits this immediately after every label | |||
| /// that is referenced by a `runto` somewhere in the test corpus. When executed, | |||
| /// checks whether the top of runtoStack targets the current address. If yes, | |||
| /// stores the program IP in programResumeIP and jumps to test_resume_ip. | |||
| /// Otherwise falls through. In `dotnet run` builds (no tests), this opcode is | |||
| /// not emitted. | |||
| /// </summary> | |||
| public const byte RUNTO_YIELD = 65; | |||
|
|
|||
| /// <summary> | |||
| /// Records an assertion failure on the VM and halts execution. The data | |||
| /// stack at the time of dispatch holds a string ptr pointing to the | |||
| /// captured source text of the asserted expression. The compiler emits | |||
| /// this only on the failure branch of an assert; passing assertions just | |||
| /// fall through. | |||
| /// </summary> | |||
| public const byte ASSERT_FAIL = 66; | |||
|
|
|||
| /// <summary> | |||
| /// Installs a void-mock for a host command. Stack at dispatch: | |||
| /// [..., commandId:int] → consumed | |||
| /// On the next CALL_HOST for that command id, the VM pops the args | |||
| /// (per CommandInfo.args metadata) but does not invoke the C# method | |||
| /// and does not push a return value. Useful for suppressing void | |||
| /// commands like `wait ms` during tests. | |||
| /// </summary> | |||
| public const byte MOCK_VOID = 67; | |||
|
|
|||
| /// <summary> | |||
| /// Installs a value-returning mock for a host command. Stack at | |||
| /// dispatch (top to bottom): | |||
| /// [..., commandId:int, returnValue:typed] → consumed | |||
| /// On the next CALL_HOST for that command id, args are popped and | |||
| /// the recorded return value is pushed in their place. | |||
| /// </summary> | |||
| public const byte MOCK_RETURNS = 68; | |||
|
|
|||
| /// <summary> | |||
| /// Installs a forbid-mock for a host command. Stack: [..., commandId:int]. | |||
| /// On dispatch, the VM records an assertion failure naming the command. | |||
| /// </summary> | |||
| public const byte MOCK_FORBID = 69; | |||
|
|
|||
| /// <summary> | |||
| /// Removes any mock registration for a single command. Stack: [..., commandId:int]. | |||
| /// </summary> | |||
| public const byte MOCK_CLEAR = 70; | |||
|
|
|||
| /// <summary> | |||
| /// Removes all mock registrations for the current VM. No stack inputs. | |||
| /// </summary> | |||
| public const byte MOCK_CLEAR_ALL = 71; | |||
|
|
|||
| /// <summary> | |||
| /// Pushes the current scope-stack depth onto the data stack as an int. | |||
| /// Depth 1 means only the global scope is live. Used by the assert-unwind | |||
| /// trampoline to walk up the scope chain draining defers, but is a | |||
| /// general primitive any future "unwind to scope N" feature can reuse. | |||
| /// </summary> | |||
| public const byte PUSH_SCOPE_DEPTH = 72; | |||
|
|
|||
| /// <summary> | |||
| /// Reads a 4-byte command id inline from the bytecode (the next 4 | |||
| /// bytes after the opcode) and pushes the current invocation count | |||
| /// for that host command onto the data stack as an int. Counts are | |||
| /// maintained on the VM regardless of whether a mock is installed, | |||
| /// so `call count cmd` works for unmocked commands too — returns 0 | |||
| /// when `cmd` was never called. | |||
| /// </summary> | |||
| public const byte CALL_COUNT = 73; | |||
|
|
|||
| /// <summary> | |||
| /// Installs a mock pointing at a bytecode body. Stack at dispatch | |||
| /// (top → bottom): commandId (int), bodyAddr (int). The VM records | |||
| /// the body's address; CALL_HOST for that command id will JUMP to | |||
| /// the body (pushing methodStack like a normal function call) so | |||
| /// the body runs with command args bound as locals in a new scope. | |||
| /// Replaces MOCK_VOID / MOCK_RETURNS / MOCK_FORBID for new mocks. | |||
| /// </summary> | |||
| public const byte MOCK_INSTALL = 74; | |||
|
|
|||
| /// <summary> | |||
| /// Writes a typed value through a PTR_REG / PTR_GLOBAL_REG pointer | |||
| /// stored in a body-local register. Reads an inline register address | |||
| /// (the body local's slot). Pops a typed value from the stack and | |||
| /// writes it through the pointer found in that local. | |||
| /// Stack at dispatch (top → bottom): typed value to write. | |||
| /// Used by ref-arg writeback at mock-body exit. | |||
| /// </summary> | |||
| public const byte WRITE_REF = 75; | |||
|
|
|||
| /// <summary> | |||
| /// Reads through a PTR_REG / PTR_GLOBAL_REG stored in a body-local | |||
| /// register and pushes the value at the pointed-to register onto | |||
| /// the data stack as a typed value. Reads an inline register | |||
| /// address (the body local's slot holding the ptr). | |||
| /// Used at mock-body prelude to initialize a ref param's value | |||
| /// register with the caller's current value. | |||
| /// </summary> | |||
| public const byte LOAD_REF = 76; | |||
|
|
|||
| /// <summary> | |||
| /// Stores a typed ref-pointer from the stack into a body-local | |||
| /// register, preserving the stack-side type code (PTR_REG or | |||
| /// PTR_GLOBAL_REG). Reads an inline register address (the slot). | |||
| /// Distinct from STORE_PTR — which reads the type code from VM | |||
| /// state — because in the mock-body prelude the VM-state typeCode | |||
| /// has been clobbered by intervening opcodes between the caller's | |||
| /// push and this store. | |||
| /// Stack at dispatch (top → bottom): typed pointer (8 bytes + type). | |||
| /// </summary> | |||
| public const byte STORE_REF = 77; | |||
|
|
|||
| /// <summary> | |||
| /// Spreads a Fade single-dimensional array onto the data stack in | |||
| /// the shape a `params` arg expects. Pops a heap pointer to the | |||
| /// array's contents (PTR_HEAP, 8 bytes + type code). Reads an | |||
| /// inline element type code (1 byte). Pushes each element as a | |||
| /// typed value in REVERSE order (so the host reads them back in | |||
| /// declaration order), then pushes the element count as an int. | |||
| /// </summary> | |||
| public const byte SPREAD_ARRAY = 78; | |||
|
|
|||
| /// <summary> | |||
| /// Pops a heap pointer (PTR_HEAP, 8 bytes + type code) and reads | |||
| /// the underlying allocation's byte length. Divides by an inline | |||
| /// element-size byte and pushes the resulting count as an int. | |||
| /// Used to implement the `len()` keyword for arrays and strings: | |||
| /// the compiler emits the element size based on the source type | |||
| /// (4 for string chars, 4/8/etc. for typed arrays). | |||
| /// </summary> | |||
| public const byte LENGTH = 79; | |||
| [Test] | ||
| public void Execute_GlobalVariables() | ||
| { | ||
| var src = @" | ||
| test foo | ||
| GLOBAL x = 32 | ||
| endtest | ||
|
|
||
| print x `this should result in an error. | ||
| "; | ||
| var (compiler, program) = Compile(src); | ||
| var vm = new VirtualMachine(program); | ||
| vm.hostMethods = compiler.methodTable; | ||
| vm.Execute3(); | ||
| } |
| Assert.That(errs.Where(e => !e.errorCode.Equals(ErrorCodes.TraverseLabelBetweenScopes)).Count(), | ||
| Is.GreaterThanOrEqualTo(0)); | ||
| } |
| [Test] | ||
| public void Len_EmptyString_ReturnsZero() | ||
| { | ||
| var src = @" | ||
| n = len("""") | ||
| "; | ||
| var vm = RunMain(src); | ||
| Assert.That(vm.error.type, Is.EqualTo(VirtualRuntimeErrorType.NONE)); | ||
| // The default for n (an int that gets assigned 0) is already 0, so | ||
| // we can't distinguish from "uninitialized" by just searching for | ||
| // the value. But no runtime error proves the path didn't trip on | ||
| // an empty allocation. | ||
| } |
|
|
||
| [Test] | ||
| public void ParseError_TypeCheck_FunctionAnd() | ||
| { | ||
| var input = @" | ||
| FUNCTION evenAndPositive(n) | ||
| isEven = n mod 2 = 0 | ||
| isPositive = n > 0 | ||
| ENDFUNCTION isEven and isPositive | ||
|
|
||
| "; | ||
| var parser = MakeParser(input); | ||
| var prog = parser.ParseProgram(); | ||
| prog.AssertNoParseErrors(); | ||
| // Assert.That(errors[0].Display, Is.EqualTo($"[1:0] - {ErrorCodes.GotoMissingLabel}")); |
| @@ -1,5 +1,6 @@ | |||
| | |||
| Microsoft Visual Studio Solution File, Format Version 12.00 | |||
| # | |||
| @@ -1,5 +1,6 @@ | |||
| | |||
| Microsoft Visual Studio Solution File, Format Version 12.00 | |||
| # | |||
| return val; | ||
| } | ||
|
|
| @@ -163,6 +163,18 @@ export async function activate(context: vscode.ExtensionContext) { | |||
| // transport: TransportKind.pipe | |||
| // } | |||
|
|
|||
| config = { | |||
| command: '/usr/local/share/dotnet/dotnet', | |||
| args: [ | |||
| 'run', | |||
| '--project', | |||
| '/Users/chrishanna/Documents/Github/dby/FadeBasic/LSP', | |||
| '--', | |||
| '--use-log-path' | |||
| ], | |||
| transport: TransportKind.pipe | |||
| } | |||
No description provided.