Skip to content
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

In node, lua require 'file', lual_dofile, luaL_loadfile, etc. doesn't seem to be working with non-empty files in jest tests #182

Open
cableray opened this issue Aug 3, 2020 · 7 comments

Comments

@cableray
Copy link

cableray commented Aug 3, 2020

So I have a lua runtime that can run luaL_dostring calls just fine. But when I call require 'other_lua_file_in_load_path' in luaL_dostring, I get the error:

(2): error loading module 'other_lua_file_in_load_path' from file './other_lua_file_in_load_path.lua':
	(null)

Similarly, loading the file with luaL_dofile, luaL_loadfile, or luaL_loadfilex (mode 't') all return an error code 2 and a null error message on the stack. This doesn't appear to be a syntax error in the file, the lua cli runs it just fine, and the file is deliberately simple. Additionally, if the file is completely empty (no newlines or anything), then the file loads fine. (but returns nothing, of course)

I'm using node v13.11.0, fengari 0.1.4, and fengari-interop 0.1.2 on OS X 10.15.5

How do I proceed in debugging this?

(Copied the following from the Jest version of this issue:)

To Reproduce

Steps to reproduce the behavior:

Try loading a lua file through Fengari in a Jest test:

global.WEB = false // not sure if this is needed
const { lua, lauxlib, lualib, to_luastring: toLuaString } = require('fengari')
const interop = require('fengari-interop')

test('Lua can load a file', () => {
    const L = lauxlib.luaL_newstate()
    lualib.luaL_openlibs(L)
    lauxlib.luaL_requiref(L, toLuaString('js'), interop.luaopen_js, 1)
    lua.lua_pop(L, 1) // remove lib from stack

  const errorCode = lauxlib.luaL_dofile(L, 'test-load.lua')
  if (errorCode) {
    throw Error(`${errorCode}: ${lua.lua_tojsstring(L, -1)}`)
  }
  expect(interop.tojs(L, -1)).toBe('file loaded')
})

file ./test-load.lua

return 'file loaded'

The result I get back is error code 2, with a null message

Expected behavior

Test passes: The result of the Lua script is returned to the jest test. (As it is with jasmine).

envinfo

  System:
    OS: macOS 10.15.5
    CPU: (8) x64 Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
  Binaries:
    Node: 13.11.0 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.13.7 - /usr/local/bin/npm
  npmPackages:
    jest: ^24.9.0 => 24.9.0 
    fengari: 0.1.4
    fengari-interop: 0.1.2

jest.config.js:

module.exports = {
  testEnvironment: 'node',
  // Exit the test suite immediately upon the first failing test suite
  bail: true,
  // Each individual test should be reported during the run
  verbose: true,
  setupFilesAfterEnv: ['jest-extended'],
  collectCoverageFrom: ['app/**/*.{js,jsx}', '!**/node_modules/**'],
  coverageReporters: ['html', 'text', 'cobertura'],
  testPathIgnorePatterns: ['/node_modules/'],
  testMatch: ['<rootDir>/test/**/?(*.)+(spec|test).js?(x)']
}
@cableray
Copy link
Author

cableray commented Aug 3, 2020

Ok, so I was testing this in a jest environment. It was having issues there, but running it directly seems to be working... That is odd...

@cableray cableray changed the title In node, lua require 'file', lual_dofile, luaL_loadfile, etc. doesn't seem to be working with non-empty files In node, lua require 'file', lual_dofile, luaL_loadfile, etc. doesn't seem to be working with non-empty files in jest tests Aug 3, 2020
@cableray
Copy link
Author

cableray commented Aug 3, 2020

Yes, confirmed that in jest tests, the code does not seem able to load files, but outside jest tests, the code works fine. So I assume there's some global state or the like that jest is messing with, but I'm not sure what that might be.

@daurnimator
Copy link
Member

fengari itself uses jest; so this seems odd...
Could you share your tests?
And please share the error message left on the stack.

@cableray
Copy link
Author

I added the details and an example project, hope that helps. There doesn't seem to be anything useful on the stack. I ran a debugger, and was looking at the buffers lua was parsing as it was loading, and they didn't seem to match my file contents. that may have been because they weren't file buffers, and I was confused though. but they were full of non-printable characters (maybe \0? I didn't check though). Hopefully you get the same result that I did when running the example. If that's not the case, then I'll see about debugging more.

@fmhdp
Copy link

fmhdp commented Apr 29, 2021

I just stumbled upon the same problem. For me it was require in lua failing that made me break out the debugger.
In the end the problem for me was a failing type check and the solution was setting the tes environment in Jest.

I've written more details on an issue filed with Jest here:
jestjs/jest#10368 (comment)

@daurnimator
Copy link
Member

daurnimator commented May 3, 2021

Thanks for digging a bit @fmhdp . This sounds like a jest issue.

However, it might be possible to rewrite https://github.com/fengari-lua/fengari/blob/master/src/lauxlib.js#L903-L944 to not use Buffer at all and tiptoe around this issue.
If you want to put in the research and work, I'm happy to review+merge it.

e.g. you could keep a Uint8Array most of the time, and use https://nodejs.org/api/buffer.html#buffer_static_method_buffer_from_arraybuffer_byteoffset_length just before each call to readSync

@fmhdp
Copy link

fmhdp commented May 3, 2021

It's good to know that fengari is still maintained and contributions are welcome. I will keep that in mind :)

Since this problem vanishes when setting the correct test environment with Jest, I'm not sure if rewriting this part is really necessary. If I can find some time for open source contributions to fengari, I'd probably focus on the io library instead, since I could use support for writing files to disk in a node environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants