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

YAML 1.2 compatibility tests #16

Open
kubkon opened this issue Nov 22, 2022 · 4 comments · May be fixed by #39
Open

YAML 1.2 compatibility tests #16

kubkon opened this issue Nov 22, 2022 · 4 comments · May be fixed by #39
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@kubkon
Copy link
Owner

kubkon commented Nov 22, 2022

It would be very cool if we subjected the parser to a YAML 1.2 compatibility tests (if such even exist).

@kubkon kubkon added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Nov 22, 2022
@adamserafini
Copy link

if such even exist).

Yep this exists:
https://matrix.yaml.info/
https://github.com/yaml/yaml-test-suite

I had thought to suggest this as well! (and add zig-yaml to the compatibility matrix) If I can ever find some time I would help setting these up.

@Bergasms
Copy link

Hello @kubkon and @adamserafini , I needed a YAML parser so I checked this out and found these issues and went down a rabbit hole.

Fixed up the parser so it runs with 0.11.0-dev.1026+4172c2916

I have now added some work to the build.zig which checks for the existence of the 'data' folder that is generated via the yaml-test-suite inside the 'test' folder. If it finds this, it then auto generates a file full of tests that look like this

`test "indent/DK95/08" {
    if(loadFromFile("test/data/tags/indent/DK95/08/in.yaml")) |yaml_const| {
        var yaml = yaml_const;
        yaml.deinit();
        try testing.expect(true);
    } else |_| {
        try testing.expect(false);
    }
}

test "indent/DK95/01" {
    if(loadFromFile("test/data/tags/indent/DK95/01/in.yaml")) |yaml_const| {
        var yaml = yaml_const;
        yaml.deinit();
        try testing.expect(false);
    } else |_| {
        try testing.expect(true);
    }
}`

Essentially just checking if we can load the YAML file without it crashing.
In the case of indent/DK95/08 it expects true if we parse correctly and false on an error.
In the case of indent/DK95/01 It expects an error (denoted by presence of 'error' file in folder).

This is all I have achieved so far, currently it sits at 349 passed; 0 skipped; 801 failed.

Feel free to check my WIP at

https://github.com/Bergasms/zig-yaml

@kubkon
Copy link
Owner Author

kubkon commented Jan 29, 2023

Very cool work @Bergasms! Wanna create a PR and upstream your work? I will happy to code review your changes. One thing that I would suggest based on the snippet you posted is for the generator to emit something like:

test "indent/DK95/08" {
    var yaml = loadFromFile("test/data/tags/indent/DK95/08/in.yaml") catch return error.Failed; // Or even hone in on the error if desired
    defer yaml.deinit();
}

test "indent/DK95/01" {
    var yaml = loadFromFile("test/data/tags/indent/DK95/01/in.yaml") catch return; // Error is success so just return
    defer yaml.deinit();
    return error.UnexpectedSuccess;
}

@Bergasms
Copy link

@kubkon Yes I will, I've been self teaching zig and failing forward, so to speak, so it will be good to have some guidance. I'll do a bit more to get it into a less 'hacked together' state and then put up a PR, when time permits.

@sencrash sencrash linked a pull request Jun 9, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants