Adds unit tests to the Keep Markup plugin

[ggrossetie]

Adds unit tests on the Keep Markup plugin using JSDOM to create a "fake" DOM.
To run the tests, open a terminal and type:

$ npm test

Sample output:

...
...
  Prism Keep Markup Plugin
    ✓ should keep <span> markup
    ✓ should preserve markup order
    ✓ should keep last <span> markup


  3 passing (44ms)

[ggrossetie]

As far as I know, this change is mandatory otherwise we can't fake self and document in the unit tests.
In a browser environment both self and document are global and will be resolved.

[mAAdhaTTah]

I need to think about this a bit. I'm not in love with mocking createRange, but if JSDOM doesn't have it yet, then maybe that's fine? Also, the test setup is in the keepmarkup test file rather than being something reusable between tests. Lastly, there's the question of whether / how we can bootstrap Prism in an isolated way between test suites.

[ggrossetie]

I need to think about this a bit. I'm not in love with mocking createRange, but if JSDOM doesn't have it yet, then maybe that's fine?

If you have a better idea, I'm more than interested! 😉

Also, the test setup is in the keepmarkup test file rather than being something reusable between tests.

I don't think we can reuse much (or anything at all).
We could reuse the "fake DOM" part but that's really just 4-5 lines of code and not all the plugins check for the same things.

For instance Custom Class checks:

if ((typeof self === 'undefined' || !self.Prism) && (typeof global === 'undefined' || !global.Prism)) {
  return;
}

But copy to clipboard checks:

if (typeof self === 'undefined' || !self.Prism || !self.document) {
  return;
}

And this plugin checks:

if (typeof self === 'undefined' || !self.Prism || !self.document || !document.createRange) {
  return;
}

Lastly, there's the question of whether / how we can bootstrap Prism in an isolated way between test suites.

Node.js has a cache but it's possible to invalidate the cache so every require is a new "instance":

delete require.cache[require.resolve('../../../prism')]
require('../../../prism')

We could create a tiny wrapper to require Prim in the tests:

const util = require('../plugin-test-util')
util.requirePrism()

[ggrossetie]

@RunDevelopment I've removed Node.js 4 and Node.js 9 (EOL) and added lts/* (Node.js 10) + node (Node.js 11 current stable release).
So Prism is building against Node.js 6, 8, 10 and 11.

[mAAdhaTTah]

I'm on board with this, and we can massage it into place as we add additional tests. I would however prefer to keep all tests in the root tests directory, which may require some adjustments. We also have removed node 4 since this PR was first opened, so we have some conflicts that need to be resolved.

[mAAdhaTTah]

I almost think snapshot tests would be better here than all these assertions. The other thing I've done is something like:

expect(result.innerHtml).to.equal('<some>html</some>');

If we didn't want to add a new dep & auto-generate them.

[ggrossetie]

I almost think snapshot tests would be better here than all these assertions. The other thing I've done is something like:

You tell me 😉
Do you know a good snapshot testing library ? In my opinion we should do what you suggest:

expect(result.innerHtml).to.equal('<some>html</some>');

Keep it simple.

[RunDevelopment]

I really like this simple approach.
We can always add a more elaborate testing scheme later on.

[mAAdhaTTah]

Great, no additional lib needed then. Alternatively: https://github.com/suchipi/chai-jest-snapshot

[RunDevelopment]

Alternatively: https://github.com/suchipi/chai-jest-snapshot

Maybe I don't get the library but don't they separate the expected result from the actual input? Wouldn't it be better to have actual and expected in the same file in our case?

[ggrossetie]

I just get back to the code and it won't work because we are using a dummy implementation of createRange (since jsdom doesn't implement it).
So innerHTML will be empty/undefined.

[ggrossetie]

Conflicts resolved and I've moved the tests to the tests directory.

[mAAdhaTTah]

Last nit: please remove the package-lock.json, as we don't have one of those at this time.

[ggrossetie]

Last nit: please remove the package-lock.json, as we don't have one of those at this time.

Done 👍

[mAAdhaTTah]

Thank you for the contribution! If you fixup #1622, we can get that in now too.