search

JetBrains / markdown

Markdown parser written in kotlin
Apache License 2.0
682 stars 75 forks source link

Add GFM and CommonMark test suites #54

Closed ajalt closed 3 years ago

ajalt commented 3 years ago

The CommonMark and GFM specs both provide comprehensive suites of test cases in the form of JSON documents that can be extracted from their spec documents.

The latest CommonMark test JSON is also available online here. The GFM test JSON can be generated from their spec. This PR creates two new test classes that use the JSON files to run all tests from the specs.

At the time of this PR, around 20% of the test cases from both suites are failing, indicating non-compliance with the specs.

By including these test suites, we can ensure that the parser and html generators are in compliance with the specifications. Since these test suites are comprehensive, this would allow us to potentially remove some of the existing hand written tests in the future.

valich commented 3 years ago

That's awesome, thanks! Unfortunately, I know there is some incompliance, but I never got to fixing it. It's a good thing there is a sane way to run checks coming, the previous approach with running spec python script was not really usable.

I'll check how this works tomorrow.

ajalt commented 3 years ago

We could potentially download the CommonMark spec JSON, but the GFM spec JSON isn't published online AFAIK. You have to generate it manually from their repo.

ajalt commented 3 years ago

Following up on https://github.com/valich/intellij-markdown/pull/55#issuecomment-719748844, w.r.t. build these tests causing CI failures.

One potential approach would be to simply skip these tests on CI until they are passing. Unfortunately it looks like kotlin.test only supports excluding tests on JVM, and not on other platforms. Another possibility would be to move these spec tests to their own separate module, which would let us run only the existing module on CI.

valich commented 3 years ago

I believe there must be a way to configure test dir patterns.. anyway, I think I have an alternative solution (hackish, unfortunately). So, I have also noticed that the test classes could (and probably should) also be generated (e.g. with a separate gradle task) and not committed to the repo. If we do that, we can make regular check task and checkWithSpec which will depend on spec test classes generation and rebuild. WDYT?

Actually, I am ready to merge the branch as it is now, but I am not sure if there's a point in a hurry. I can use it to find and fix failures already (thanks for that!), and maybe I should just make fixing tests the first priority instead.

ajalt commented 3 years ago

The CommonMark and GFM specs are pretty much complete by now. The CommonMark spec has only changed once in the past three years. We could write code to generate the tests, but I don't know if it's worth it given how infrequently they change.

ajalt commented 3 years ago

I added gradle tasks that will clone the commonmark and gfm repos, run their python scripts to generate the json tests, then use that json to generate kotlin tests directly that can be checked in. This means we don't have to check in the json files, and the generated tests could run on iOS an the browser.

I marked both generated test files as @Ignore for now so that they can be merged without affecting CI. You can remove those annotations to see the test failures.

valich commented 3 years ago

I've resolved conflicts manually and pushed commits into master branch, so this one's merged. Thanks!