Snapshot testing

[czosel]

Currently, a typical test looks roughly like this:

  it("test coalesce operator", function() {
    var ast = parser.parseEval("$var = $a ?? true;");
    ast.children[0].right.kind.should.be.exactly("bin");
    ast.children[0].right.type.should.be.exactly("??");
    ast.children[0].right.left.kind.should.be.exactly("variable");
    ast.children[0].right.right.kind.should.be.exactly("boolean");
});

After working with snapshot-based testing for a while in prettier/plugin-php, I've really come to appreciate not having to write my own assertions anymore.

If we were to introduce snapshot-based testing here, the test case above would essentialy boil down to just one line, which could even be stored in a .php file:

$var = $a ?? true;

The testing framework would then generate the snapshot and save it in a file:

{
  "kind": "program",
  "children": [
    {
      "kind": "assign",
      "operator": "=",
      "left": {
        "kind": "variable",
        "name": "var",
        "byref": false,
        "curly": false
      },
      "right": {
        "kind": "bin",
        "type": "??",
        "left": {
          "kind": "variable",
          "name": "a",
          "byref": false,
          "curly": false
        },
        "right": {
          "kind": "boolean",
          "value": true,
          "raw": "true"
        }
      }
    }
  ],
  "errors": []
}

Like this, we'd see the exact AST changes that every PR introduces to any of the existing test cases.

It seems that Esprima is using a similar approach.

If you're interested, I could try preparing a little proof of concept - let me know what you think! 😄

[ichiriac]

Hi @czosel

I like the idea of make tests based on snapshots, I miss them here over the AST and it could help to increase easily my code coverage.

By the way, I've actually done some work in the same direction based on tokens :

• here the testing framework : https://github.com/glayzzle/php-parser-dev
• here snapshoots of various frameworks : https://github.com/glayzzle/php-parser-dev-tokens

Sadly I have not enough time to work on the project but I would be very glad if you could help me with that.

[czosel]

Hi @ichiriac, thanks for your quick response - I didn't know about the two testing repos yet. I guess that integrating snapshot testing in this repo and basically porting the existing test cases would already be a great first step.

I hope I'll find the time soon to give this a shot!

[ichiriac]

Hi @czosel,

I've tried a simplified version that scans the fixtures folder and builds tests from it. Maybe we should have a separate folder between AST and tokens.

Related to #49, we could start by migrate actual tests to fixtures, it's a lot of work to be done.

/cc @b4dnewz, @DaGhostman if you have time and want to help, it would be highly apreciated

[DaGhostman]

Would love to be of help, I'll try my best to find some spare time and help with whatever I can :)

[b4dnewz]

hi everybody, I looked to this snapshot testing and I think is a wonderful idea!
I've a suggestion regarding the fixture.js file, right now if the parsing produces an error it still write the file, I guess is like so for testing the errors too, am i right?

otherwise my proposal is to write the .json output only if the parsing succeed so you don't have to manually remove it after you fix the php file template issue.

try {
  // parse the file
  ast = parser.parseCode(buffer, filename, options);
  ast = JSON.stringify(ast, null, 2);

  try {
    original = fs.readFileSync(filename + ".json").toString();
  } catch (e) {
    console.log("Generating snapshot: " + filename);
    // GENERATE THE OUTPUT (IF FILE DOES NOT EXISTS)
    original = ast;
    fs.writeFileSync(filename + ".json", original);
  }

  original.should.be.exactly(ast, "Fail " + filename);
} catch (e) {
  console.log("Failed file parsing for:", filename);
  console.error(e);
}

otherwise if is needed for testing errors it's good as it is.

also I see is a very huge work to do and I would like some tips from @ichiriac about where to start and how to deal these tests when there are more than one ast parsing for describe method like this one without loosing the readability of the tests.

maybe a recursive fixture.js function that look for subfolders and creates describe blocks.
by the way, this snapshot testing is wery cool! 😎

[czosel]

@b4dnewz I'd suggest using an existing testing framework to do this, for example jest. I wanted to prepare a little proof of concept, but didn't find the time yet 😞

[b4dnewz]

ah, ok! I agree

[ichiriac]

Hi @b4dnewz, glad to see you 😄

I've wrote this quick (& dirty) without handling errors types in order to draft something. I did not know about Jest but it's a more clean way to generate snapshot. We need to handle parsing errors also, maybe with the supressErrors option in order to have more details.

I can try to bootstrap something with Jest, and @czosel you could check if it's the right way to use it. If we are good to go, @b4dnewz you could start to migrate the code, it should be easy as the only thing to do is to remove all asserts and replace them with a single match assert.

The next hard step where everybody can help is to check the results on the code coverage, find uncovered use cases, and trigger them code. This part is hard as sometimes it's tricky to find a way to create the specific code or reproduce edge cases.

[ichiriac]

Hi,

I've started a new branch here, and switched all tests over jest : https://github.com/glayzzle/php-parser/tree/v3.1

You can make your PR there :)

[ichiriac]

thanks @b4dnewz, I've merged your PR #157, all tests are under Jest and the code coverage is great, you've done an amazing work !

Many thanks dude 👍

[ichiriac]

This fix is now relased under 3.0.0-alpha.3