Boa: Refactor the JS testing system

[dvtkrlbs]

Currently most of the tests are just the same boilerplate:

#[test]
fn test() {
    let mut ctx = Context::new();
    let init = r#"some js"#;
    eprintln!("{}", forward(&mut ctx, init));

    assert_eq!(forward(&mut ctx, "value we expect from execution of that js code");
}

I think these codes can be extracted to js files and be executed by some test runner. V8 does this by creating js file and an out file that contains the expected output. Also another way is exposing some testing related functions like assert to js side while testing and doing this with single file. I realize that this is similiar to the 262 tests.

[Lan2u]

As you say this seems to very much overlap with the work required for test 262, it makes sense to me that we could utilise the existing progress into that (e.g. the gh page display etc.) for this.

[Razican]

There is some debate on this. On one hand, we can reduce the boilerplate needed by providing a check_output() function that receives some code and the expected output, and that might work for many of our tests. This would be in charge of creating the context and doing the asserts and so on.

Then, there is the thing about moving things to JS files. We have already done this with benchmarks, and they are still compiled and executed fast (which is the main concern with the Test262 suite: it takes around 10 minutes to run). This would also allow running only one test, for example. The main issue I see with this is keeping track of the input/output files and tests, and making sure all of them are run.

Something that could be done in this regard would be to implement some kind of "test creator" that would generate code for all the tests in a given folder. This, of course requires some extra work, and a decision has to be taken on the way we implement it. This, as @Lan2u says, might take over the work done by the Test262 tester, and might be slower. We must also ensure a way for these tests to be created before every commit.

Let's see what others think @HalidOdat @jasonwilliams @RageKnify

[RageKnify]

This would also allow running only one test, for example.

cargo already allows this, running cargo test $TOP::$MID::$LOWER::tests::$TEST_NAMElets you run a single test, you can also give the name of a module to run all the tests from that module.

While there is a bit of boiler-plate around each test I don't dislike the current setup.
Not sure how we'd be able to get coverage values with the different setup.

[jasonwilliams]

On one hand, we can reduce the boilerplate needed by providing a check_output() function that receives some code and the expected output, and that might work for many of our tests. This would be in charge of creating the context and doing the asserts and so on.

I would much prefer us starting with this then assessing from there before we start making a load of boilerplate.

[bartlomieju]

Following up on this issue, I believe a simple helper like below could help significantly reduce boilerplate and repetitions for most common test cases.

fn check_output(maybe_init: Option<&str>, cases: Vec<(String, String)>) {
    let mut context = Context::new();

    if let Some(init) = maybe_init {
        forward(&mut context, init);
    }

    for (case, expected) in cases {
        assert_eq!(forward(&mut context, case), expected);
    }
}

Example tests before and after:

// before
#[test]
fn repeat() {
    let mut context = Context::new();
    let init = r#"
        var empty = new String('');
        var en = new String('english');
        var zh = new String('中文');
        "#;

    forward(&mut context, init);

    assert_eq!(forward(&mut context, "empty.repeat(0)"), "\"\"");
    assert_eq!(forward(&mut context, "empty.repeat(1)"), "\"\"");

    assert_eq!(forward(&mut context, "en.repeat(0)"), "\"\"");
    assert_eq!(forward(&mut context, "zh.repeat(0)"), "\"\"");

    assert_eq!(forward(&mut context, "en.repeat(1)"), "\"english\"");
    assert_eq!(forward(&mut context, "zh.repeat(2)"), "\"中文中文\"");
}

// after
#[test]
fn repeat() {
    let init = r#"
        var empty = new String('');
        var en = new String('english');
        var zh = new String('中文');
        "#;

    check_output(Some(init), vec![
        ("empty.repeat(0)", "\"\""),
        ("empty.repeat(1)", "\"\""),

        ("en.repeat(0)", "\"\""),
        ("zh.repeat(0)", "\"\""),

        ("en.repeat(1)", "\"english\""),
        ("zh.repeat(2)", "\"中文中文\""),
    ]);
}

#[test]
fn as_uint_n() {
    let mut context = Context::new();

    assert_eq!(forward(&mut context, "BigInt.asUintN(0, -2n)"), "0n");
    assert_eq!(forward(&mut context, "BigInt.asUintN(0, -1n)"), "0n");
    assert_eq!(forward(&mut context, "BigInt.asUintN(0, 0n)"), "0n");
    assert_eq!(forward(&mut context, "BigInt.asUintN(0, 1n)"), "0n");
    assert_eq!(forward(&mut context, "BigInt.asUintN(0, 2n)"), "0n");

    assert_eq!(forward(&mut context, "BigInt.asUintN(1, -3n)"), "1n");
    assert_eq!(forward(&mut context, "BigInt.asUintN(1, -2n)"), "0n");
    assert_eq!(forward(&mut context, "BigInt.asUintN(1, -1n)"), "1n");
    assert_eq!(forward(&mut context, "BigInt.asUintN(1, 0n)"), "0n");
    assert_eq!(forward(&mut context, "BigInt.asUintN(1, 1n)"), "1n");
    assert_eq!(forward(&mut context, "BigInt.asUintN(1, 2n)"), "0n");
    assert_eq!(forward(&mut context, "BigInt.asUintN(1, 3n)"), "1n");

    assert_eq!(
        forward(&mut context, "BigInt.asUintN(1, -123456789012345678901n)"),
        "1n"
    );
    assert_eq!(
        forward(&mut context, "BigInt.asUintN(1, -123456789012345678900n)"),
        "0n"
    );
    assert_eq!(
        forward(&mut context, "BigInt.asUintN(1, 123456789012345678900n)"),
        "0n"
    );
    assert_eq!(
        forward(&mut context, "BigInt.asUintN(1, 123456789012345678901n)"),
        "1n"
    );

    assert_eq!(
        forward(
            &mut context,
            "BigInt.asUintN(200, 0xbffffffffffffffffffffffffffffffffffffffffffffffffffn)"
        ),
        "1606938044258990275541962092341162602522202993782792835301375n"
    );
    assert_eq!(
        forward(
            &mut context,
            "BigInt.asUintN(201, 0xbffffffffffffffffffffffffffffffffffffffffffffffffffn)"
        ),
        "3213876088517980551083924184682325205044405987565585670602751n"
    );
}

#[test]
fn as_uint_n() {
    check_output(None, vec![
        ("BigInt.asUintN(0, -2n)", "0n"),
        ("BigInt.asUintN(0, -1n)", "0n"),
        ("BigInt.asUintN(0, 0n)", "0n"),
        ("BigInt.asUintN(0, 1n)", "0n"),
        ("BigInt.asUintN(0, 2n)", "0n"),

        ("BigInt.asUintN(1, -3n)", "1n"),
        ("BigInt.asUintN(1, -2n)", "0n"),
        ("BigInt.asUintN(1, -1n)", "1n"),
        ("BigInt.asUintN(1, 0n)", "0n"),
        ("BigInt.asUintN(1, 1n)", "1n"),
        ("BigInt.asUintN(1, 2n)", "0n"),
        ("BigInt.asUintN(1, 3n)", "1n"),

        ("BigInt.asUintN(1, -123456789012345678901n)", "1n"),
        ("BigInt.asUintN(1, -123456789012345678900n)", "0n"),
        ("BigInt.asUintN(1, 123456789012345678900n)", "0n"),
        ("BigInt.asUintN(1, 123456789012345678901n)", "1n"),

        (
            "BigInt.asUintN(200, 0xbffffffffffffffffffffffffffffffffffffffffffffffffffn)",
            "1606938044258990275541962092341162602522202993782792835301375n"
        ),
        (
            "BigInt.asUintN(201, 0xbffffffffffffffffffffffffffffffffffffffffffffffffffn)",
            "3213876088517980551083924184682325205044405987565585670602751n"
        ),
    ]);
}

[jasonwilliams]

Looks good @bartlomieju would be happy seeing this change

[bartlomieju]

Looks good @bartlomieju would be happy seeing this change

Sure, I'll open a PR that adds this helper and uses it in a few testing files.

[HalidOdat]

@bartlomieju Looks good :)

I just have some minor improvements:
- Use string slices &str, instead of allocated strings.
- Use slices, instead of allocated Vec (so we can use stack allocated array)
- Add #[track_caller] so we know where which test failed.
- Use use message argument of assert_eq! so we know which test case failed.

#[track_caller]
fn check_output(maybe_init: Option<&str>, cases: &[(&str, &str)]) {
    let mut context = Context::new();

    if let Some(init) = maybe_init {
        forward(&mut context, init);
    }

    for (i, (case, expected)) in cases.enumerate() {
        assert_eq!(forward(&mut context, case), expected, "Test case {} ('{}')", i + 1, case);
    }
}

[HalidOdat]

This has been solved by #1458