APT-1523 fallback to loadstring if loadmodule not enabled (#392)

Roblox · Aug 16, 2024 · 70f4243 · 70f4243
1 parent 60586d7
commit 70f4243
Show file tree

Hide file tree

Showing 15 changed files with 96 additions and 32 deletions.
diff --git a/bin/ci.sh b/bin/ci.sh
@@ -14,4 +14,10 @@ robloxdev-cli run --load.model jest.project.json \
   EnableSignalBehavior=true DebugForceDeferredSignalBehavior=true MaxDeferReentrancyDepth=15 \
   --lua.globals=UPDATESNAPSHOT=false --lua.globals=CI=false --load.asRobloxScript --fs.readwrite="$(pwd)"
 
+echo "Running low privilege tests"
+robloxdev-cli run --load.model jest.project.json \
+  --run bin/spec.lua --testService.errorExitCode=1 \
+  --fastFlags.overrides EnableLoadModule=false --load.asRobloxScript
+
+# Uncomment this to update snapshots
 # robloxdev-cli run --load.model jest.project.json --run bin/spec.lua --testService.errorExitCode=1 --fastFlags.allOnLuau --fastFlags.overrides EnableLoadModule=true DebugDisableOptimizedBytecode=true  --lua.globals=UPDATESNAPSHOT=true --lua.globals=CI=true --load.asRobloxScript --fs.readwrite="$(pwd)"
diff --git a/src/jest-circus/src/circus/__tests__/__snapshots__/errorParsing.roblox.spec.snap.lua b/src/jest-circus/src/circus/__tests__/__snapshots__/errorParsing.roblox.spec.snap.lua
@@ -19,7 +19,7 @@ exports[ [=[formats an error object into proper output with message 1]=] ] = [=[
 Table {
   "Error
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.errorParsing.roblox.spec:43
-LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:2046 function _execModule
+LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:2051 function _execModule
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1414 function _loadModule
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1260
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1259 function requireModule
@@ -33,7 +33,7 @@ exports[ [=[formats an error object with a message into proper output with messa
 Table {
   "Error: something went wrong!!
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.errorParsing.roblox.spec:44
-LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:2046 function _execModule
+LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:2051 function _execModule
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1414 function _loadModule
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1260
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1259 function requireModule
@@ -47,7 +47,7 @@ exports[ [=[formats an error object with a stack and message into proper output
 Table {
   "something went wrong!!
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.errorParsing.roblox.spec:47
-LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:2046 function _execModule
+LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:2051 function _execModule
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1414 function _loadModule
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1260
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1259 function requireModule
@@ -61,7 +61,7 @@ exports[ [=[formats an error object with only a stack into proper output with me
 Table {
   "Error: something went wrong!!
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.errorParsing.roblox.spec:51
-LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:2046 function _execModule
+LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:2051 function _execModule
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1414 function _loadModule
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1260
 LoadedCode.JestRoblox._Workspace.JestRuntime.JestRuntime:1259 function requireModule

diff --git a/...est-circus/src/circus/__tests__/__snapshots__/formatNodeAssertErrors.roblox.spec.snap.lua b/...est-circus/src/circus/__tests__/__snapshots__/formatNodeAssertErrors.roblox.spec.snap.lua
@@ -16,8 +16,6 @@ Difference:
 <dim>Compared values have no visual difference.</>
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:39
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.utils:369
-LoadedCode.JestRoblox._Index.Promise.Promise:172 function runExecutor
-LoadedCode.JestRoblox._Index.Promise.Promise:299
 ",
   },
 }
@@ -43,7 +41,7 @@ Difference:
 <green>-   \"foo\": \"foo\",</>
 <red>+   \"foo\": \"fooBar\",</>
 <dim>  }</>
-LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:117
+LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:118
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.utils:369
 ",
   },
@@ -70,7 +68,7 @@ Difference:
 <green>-   \"foo\": \"foo\",</>
 <red>+   \"foo\": \"fooBar\",</>
 <dim>  }</>
-LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:117
+LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:118
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.utils:369
 ",
   },
@@ -91,7 +89,7 @@ Table {
 Difference:
 
 <dim>Compared values have no visual difference.</>
-LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:117
+LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:118
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.utils:369
 ",
   },
@@ -112,7 +110,7 @@ Table {
 Difference:
 
 <dim>Compared values have no visual difference.</>
-LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:117
+LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:118
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.utils:369
 ",
   },
@@ -133,7 +131,7 @@ Table {
 Difference:
 
 <dim>Compared values have no visual difference.</>
-LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:117
+LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:118
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.utils:369
 ",
   },
@@ -154,7 +152,7 @@ Table {
 Difference:
 
 <dim>Compared values have no visual difference.</>
-LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:117
+LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:118
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.utils:369
 ",
   },
@@ -175,7 +173,7 @@ Table {
 Difference:
 
 <dim>Compared values have no visual difference.</>
-LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:117
+LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:118
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.utils:369
 ",
   },
@@ -196,7 +194,7 @@ Table {
 Difference:
 
 <dim>Compared values have no visual difference.</>
-LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:117
+LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:118
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.utils:369
 ",
   },
@@ -223,7 +221,7 @@ Difference:
 <green>-   \"foo\": \"foo\",</>
 <red>+   \"foo\": \"fooBar\",</>
 <dim>  }</>
-LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:117
+LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:118
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.utils:369
 ",
   },
@@ -250,7 +248,7 @@ Difference:
 <green>-   \"foo\": \"foo\",</>
 <red>+   \"foo\": \"fooBar\",</>
 <dim>  }</>
-LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:117
+LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.__tests__.formatNodeAssertErrors.roblox.spec:118
 LoadedCode.JestRoblox._Workspace.JestCircus.JestCircus.circus.utils:369
 ",
   },

diff --git a/src/jest-circus/src/circus/__tests__/afterAll.spec.lua b/src/jest-circus/src/circus/__tests__/afterAll.spec.lua
@@ -15,6 +15,12 @@ local JestGlobals = require(Packages.Dev.JestGlobals)
 local expect = JestGlobals.expect
 local it = JestGlobals.it
 
+-- ROBLOX deviation: these tests require loadmodule
+local loadModuleEnabled = pcall((debug :: any).loadmodule, Instance.new("ModuleScript"))
+if not loadModuleEnabled then
+	it = it.skip :: any
+end
+
 it("tests are not marked done until their parent afterAll runs", function()
 	local stdout = runTest([[
 			describe("describe", function()

diff --git a/src/jest-circus/src/circus/__tests__/baseTest.spec.lua b/src/jest-circus/src/circus/__tests__/baseTest.spec.lua
@@ -15,6 +15,12 @@ local JestGlobals = require(Packages.Dev.JestGlobals)
 local expect = JestGlobals.expect
 local test = JestGlobals.test
 
+-- ROBLOX deviation: these tests require loadmodule
+local loadModuleEnabled = pcall((debug :: any).loadmodule, Instance.new("ModuleScript"))
+if not loadModuleEnabled then
+	test = test.skip :: any
+end
+
 test("simple test", function()
 	local stdout = runTest([[
 			describe("describe", function()
@@ -28,15 +34,15 @@ test("simple test", function()
 end)
 -- ROBLOX deviation START: see if we can make this work later
 -- test("function descriptors", function()
-test.skip("function descriptors", function()
-	-- ROBLOX deviation END
-	local stdout = runTest([[
-		describe(function describer() end, function()
-			test(class One {}, function() end);
-		end)
-  	]]).stdout
-	expect(stdout).toMatchSnapshot()
-end)
+-- test.skip("function descriptors", function()
+-- 	-- ROBLOX deviation END
+-- 	local stdout = runTest([[
+-- 		describe(function describer() end, function()
+-- 			test(class One {}, function() end);
+-- 		end)
+--   	]]).stdout
+-- 	expect(stdout).toMatchSnapshot()
+-- end)
 test("failures", function()
 	local stdout = runTest([[
 			describe("describe", function()

diff --git a/src/jest-circus/src/circus/__tests__/formatNodeAssertErrors.roblox.spec.lua b/src/jest-circus/src/circus/__tests__/formatNodeAssertErrors.roblox.spec.lua
@@ -50,6 +50,7 @@ describe("formatNodeAssertErrors", function()
 			parent = {} :: any,
 			seenDone = false,
 		}
+		assertionError.stack = pruneDeps(assertionError.stack)
 		formatNodeAssertErrors(nil, {
 			name = "test_done",
 			test = test,

diff --git a/src/jest-circus/src/circus/__tests__/hooks.spec.lua b/src/jest-circus/src/circus/__tests__/hooks.spec.lua
@@ -15,6 +15,12 @@ local JestGlobals = require(Packages.Dev.JestGlobals)
 local expect = JestGlobals.expect
 local it = JestGlobals.it
 
+-- ROBLOX deviation: these tests require loadmodule
+local loadModuleEnabled = pcall((debug :: any).loadmodule, Instance.new("ModuleScript"))
+if not loadModuleEnabled then
+	it = it.skip :: any
+end
+
 it("beforeEach is executed before each test in current/child describe blocks", function()
 	local stdout = runTest([[
 			describe("describe", function()

diff --git a/src/jest-circus/src/circus/formatNodeAssertErrors.lua b/src/jest-circus/src/circus/formatNodeAssertErrors.lua
@@ -18,6 +18,7 @@ local RobloxShared = require(Packages.RobloxShared)
 local escapePatternCharacters = RobloxShared.escapePatternCharacters
 local normalizePromiseError = RobloxShared.normalizePromiseError
 local Error = LuauPolyfill.Error
+local cleanLoadStringStack = RobloxShared.cleanLoadStringStack
 -- ROBLOX deviation END
 
 type Record<K, T> = { [K]: T }

diff --git a/src/jest-message-util/src/init.lua b/src/jest-message-util/src/init.lua
@@ -30,7 +30,9 @@ local prettyFormat = require(Packages.PrettyFormat).format
 type Path = Config_Path
 
 -- ROBLOX deviation START: additional dependencies
-local normalizePromiseError = require(Packages.RobloxShared).normalizePromiseError
+local RobloxShared = require(Packages.RobloxShared)
+local normalizePromiseError = RobloxShared.normalizePromiseError
+local cleanLoadStringStack = RobloxShared.cleanLoadStringStack
 -- ROBLOX deviation END
 
 -- ROBLOX deviation: forward declarations
@@ -275,8 +277,8 @@ end
 
 -- ROBLOX deviation: config does not have StackTraceConfig type annotation
 local function formatPaths(config, relativeTestPath, line: string): string
-	-- ROBLOX deviation: we don't do any formatting of paths in Lua to align with upstream
-	return line
+	-- ROBLOX deviation: if loadstring is used, format the loadstring stacktrace to look like a path
+	return cleanLoadStringStack(line)
 end
 
 function getStackTraceLines(stack: string, options: StackTraceOptions): { string }

diff --git a/src/jest-mock/src/init.lua b/src/jest-mock/src/init.lua
@@ -20,7 +20,6 @@ local LuauPolyfill = require(Packages.LuauPolyfill)
 local Array = LuauPolyfill.Array
 local Boolean = LuauPolyfill.Boolean
 local Error = LuauPolyfill.Error
-local Object = LuauPolyfill.Object
 local Set = LuauPolyfill.Set
 local Symbol = LuauPolyfill.Symbol
 

diff --git a/src/jest-runtime/src/init.lua b/src/jest-runtime/src/init.lua
@@ -1943,6 +1943,7 @@ function Runtime_private:_execModule(
 	local moduleFunction, defaultEnvironment, errorMessage, cleanupFn
 
 	local modulePath = localModule.filename
+	local loadModuleEnabled = pcall((debug :: any).loadmodule, Instance.new("ModuleScript"))
 
 	if self._loadedModuleFns and self._loadedModuleFns:has(modulePath) then
 		local loadedModule = self._loadedModuleFns:get(modulePath) :: { any }
@@ -1951,8 +1952,12 @@ function Runtime_private:_execModule(
 	else
 		-- Narrowing this type here lets us appease the type checker while still
 		-- counting on types for the rest of this file
-		local loadmodule: (ModuleScript) -> (any, string, () -> any) = debug["loadmodule"]
-		moduleFunction, errorMessage, cleanupFn = loadmodule(modulePath)
+		if loadModuleEnabled then
+			local loadmodule: (ModuleScript) -> (any, string, () -> any) = debug["loadmodule"]
+			moduleFunction, errorMessage, cleanupFn = loadmodule(modulePath)
+		else
+			moduleFunction = loadstring(modulePath.Source, modulePath:GetFullName())
+		end
 		-- ROBLOX NOTE: we are not using assert() as it throws a bare string and we need to throw an Error object
 		if moduleFunction == nil then
 			error(Error.new(errorMessage))
@@ -1990,7 +1995,7 @@ function Runtime_private:_execModule(
 			Adding `script` directly into a table so that it is accessible to the debugger
 			It seems to be a similar issue to code inside of __index function not being debuggable
 		]]
-		script = defaultEnvironment.script,
+		script = if loadModuleEnabled then defaultEnvironment.script else modulePath,
 		require = if isInternal
 			then function(scriptInstance: ModuleScript)
 				return self:requireInternalModule(scriptInstance)

diff --git a/src/jest-snapshot/src/init.lua b/src/jest-snapshot/src/init.lua
@@ -16,6 +16,11 @@ local Error = LuauPolyfill.Error
 local instanceof = LuauPolyfill.instanceof
 local AssertionError = LuauPolyfill.AssertionError
 
+-- ROBLOX deviation START: additional dependencies
+local RobloxShared = require(Packages.RobloxShared)
+local cleanLoadStringStack = RobloxShared.cleanLoadStringStack
+-- ROBLOX deviation END
+
 local getType = require(Packages.JestGetType).getType
 
 -- ROBLOX TODO: ADO-1633 fix Jest Types imports
@@ -410,6 +415,9 @@ function _toThrowErrorMatchingSnapshot(config: types.MatchSnapshotConfig, fromPr
 		error_ = tostring(error_)
 	end
 
+	-- ROBLOX deviation: if loadstring is used, format the loadstring stacktrace to look like a path
+	error_ = cleanLoadStringStack(error_)
+
 	return _toMatchSnapshot({
 		context = context,
 		hint = hint,

diff --git a/src/roblox-shared/src/cleanLoadStringStack.lua b/src/roblox-shared/src/cleanLoadStringStack.lua
@@ -0,0 +1,21 @@
+local loadModuleEnabled = pcall((debug :: any).loadmodule, Instance.new("ModuleScript"))
+
+return function(line: string): string
+	if not loadModuleEnabled then
+		local spacing, filePath, lineNumber, extra = line:match('(%s*)%[string "(.-)"%]:(%d+)(.*)')
+		if filePath then
+			local match = filePath
+			if spacing then
+				match = spacing .. match
+			end
+			if lineNumber then
+				match = match .. ":" .. lineNumber
+			end
+			if extra then
+				match = match .. extra
+			end
+			return match
+		end
+	end
+	return line
+end
diff --git a/src/roblox-shared/src/init.lua b/src/roblox-shared/src/init.lua
@@ -17,6 +17,7 @@ local CurrentModule = script
 local nodeUtilsModule = require(CurrentModule.nodeUtils)
 export type NodeJS_WriteStream = nodeUtilsModule.NodeJS_WriteStream
 local exports = {
+	cleanLoadStringStack = require(CurrentModule.cleanLoadStringStack),
 	dedent = require(CurrentModule.dedent).dedent,
 	escapePatternCharacters = require(CurrentModule.escapePatternCharacters).escapePatternCharacters,
 	ensureDirectoryExists = require(CurrentModule.ensureDirectoryExists),

diff --git a/src/roblox-shared/src/pruneDeps.lua b/src/roblox-shared/src/pruneDeps.lua
@@ -12,6 +12,9 @@
 	* See the License for the specific language governing permissions and
 	* limitations under the License.
 ]]
+
+local cleanLoadStringStack = require(script.Parent.cleanLoadStringStack)
+
 local function pruneDeps(str: string?): string?
 	if str == nil then
 		return nil
@@ -22,6 +25,7 @@ local function pruneDeps(str: string?): string?
 		if line:find("LoadedCode.JestRoblox._Index.") then
 			continue
 		end
+		line = cleanLoadStringStack(line)
 		table.insert(newLines, line)
 	end
 	return table.concat(newLines, "\n")