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

Fix Failing Python Tests #3717

Closed
theduke opened this issue Mar 28, 2023 · 4 comments
Closed

Fix Failing Python Tests #3717

theduke opened this issue Mar 28, 2023 · 4 comments
Assignees
Labels
📦 lib-wasi About wasmer-wasi lib-wasix Issues related to the WASIX toolchain. priority-high High priority issue
Milestone

Comments

@theduke
Copy link
Contributor

theduke commented Mar 28, 2023

A few Python/webc file system related tests were disabled to get the CI green again.

Need to fix those and re-enable the tests.

@theduke theduke added the priority-high High priority issue label Mar 28, 2023
@theduke theduke added this to the v3.2 milestone Mar 28, 2023
theduke added a commit that referenced this issue Mar 28, 2023
Ignore some tests that are currently failing.

Fixing them is tracked in #3717
Michael-F-Bryan pushed a commit that referenced this issue Apr 3, 2023
@Michael-F-Bryan
Copy link
Contributor

After some more investigation with @syrusakbary and @theduke (notes), it seems like this isn't a problem with the runners or our overlay filesystem because we were able to run the equivalent code by mounting Python's files from the host and doing wasmer run ./python.wasm directly. There seems to be a bug in our WASI implementation.

@theduke was able to reproduce this fairly consistently:

Test case
  #[test]
fn test_python() {
    let engine = wasmer::Engine::default();
    let contents = std::fs::read("/home/theduke/tmp/python/atoms/python").unwrap();
    let module = wasmer::Module::new(&engine, &contents).unwrap();

    let root_fs = RootFileSystemBuilder::new()
        .with_tty(Box::new(DeviceFile::new(__WASI_STDIN_FILENO)))
        .build();
    let fs_backing: Arc<dyn FileSystem + Send + Sync> =
        Arc::new(PassthruFileSystem::new(default_fs_backing()));
    root_fs
        .mount(
            "/lib".into(),
            &fs_backing,
            "/home/theduke/tmp/python/atom/lib".into(),
        )
        .unwrap();

    // Open the root of the new filesystem
    let err = WasiEnvBuilder::new("python")
        .args(&["-c", "import sys; sys.exit(88);"])
        .sandbox_fs(root_fs)
        .preopen_dir(Path::new("/"))
        .unwrap()
        .map_dir(".", "/")
        .unwrap()
        .run(module)
        .err()
        .unwrap();

    dbg!(&err);

    if let Some(88) = err.as_exit_code().map(|x| x.raw()) {
    } else {
        panic!("{}", err.to_string());
    }
}

Note that the issue itself is non-deterministic. You might see it error out the first time you run the test, or you could continually run the test for 5+ minutes and not see any issues. Either way, the test would always fail with the following:

Fatal Python error: Py_Initialize: can't initialize sys standard streams
LookupError: unknown encoding: ascii

We believe this happens because the Python interpreter can't import the ascii module from /lib/python3.6/encodings.

We are also seeing similar issues with the WordPress demo, except the SQLite database won't load properly and WordPress will continually prompt you to re-install. This doesn't seem to be a problem with WebcFileSystem or the overlay fs being readonly because you can mount the database folder from your host and see the same error.

I've added some integration tests to the unknown-wasi-fs-bug branch that we can use. You can test the issue using this command:

$ export RUST_BACKTRACE=1 
$ while true; do
    clear
    cargo t --package wasmer-wasix --test bug_3717 -- test_python 2>&1 > output.txt || break
done

The combined stdout, stderr, and tracing logs will be saved to output.txt.

@Michael-F-Bryan Michael-F-Bryan added 📦 lib-wasi About wasmer-wasi lib-wasix Issues related to the WASIX toolchain. labels Apr 3, 2023
@Michael-F-Bryan
Copy link
Contributor

Our go/no-go gauge is being able to run the WordPress demo using wasmer run-unstable. An easy way to test this is by checking out the minimal branch and running the following:

$ cd new
$ /wherever/target/release/wasmer run-unstable .. --mapdir=/db:../db

You should see a "Disk I/O Error" message when you browse to http://localhost:8000/. Running the same thing with php -S localhost:8000 on the host and following these instructions should dump the database contents.

@ptitSeb ptitSeb modified the milestones: v3.2, v3.3 Apr 13, 2023
@ptitSeb
Copy link
Contributor

ptitSeb commented Apr 13, 2023

There is still some flacky issue on the CI, around python. So this ticket is not 100% fixed.
however, Wordpress works now, so remining issue can be fixed on 3.3

@ptitSeb
Copy link
Contributor

ptitSeb commented Apr 20, 2023

fixed #3767

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 lib-wasi About wasmer-wasi lib-wasix Issues related to the WASIX toolchain. priority-high High priority issue
Projects
None yet
Development

No branches or pull requests

5 participants