WIP: test: jasmine-ify original tests #1129

davisjam · 2018-03-04T20:12:45Z

Jasmine-ifying original tests.

So far:

amsps_and_angles_encoding
auto_links
links_inline_style
inline_html_simple

Test(s) exist to ensure functionality and minimize regresstion (if no tests added, list tests covering this PR); or,
no tests required for this PR.
If submitting new feature, it has been documented in the appropriate places.

UziTech · 2018-03-04T21:53:25Z

test/integration/marked-spec.js

+describe('integration suite', function () {
+  it('should put plaintext into a paragraph', function () {
+    expect(marked('Hello World!')).toBe('<p>Hello World!</p>\n');
+  });


We don't need to keep this test if we don't want to. I just added it as an example

Eventually it'll go under a describe('plaintext') or something.

These are based on test/original/links_inline_style.md

davisjam · 2018-03-04T22:08:19Z

a693f23: implements suggestion from 1121. npm test will now run the old tests first.

davisjam · 2018-03-04T22:10:00Z

@UziTech

So these tests will be prettier if we can write code like this:

expect(marked(`[${desc.text}](${desc.link})`)).toBe(`<p><a href="${des

which is not "old node"-compliant (hence the travis failure).

Thoughts?

UziTech · 2018-03-04T22:17:19Z

Ya we need to stick to es5 in the tests until we figure out a build step or drop node <6

UziTech · 2018-03-04T22:18:27Z

I think the idea is v2.0 we will support only evergreen browsers and node >=6. At that time we will switch everything to ES6+

joshbruce · 2018-03-04T22:19:00Z

@davisjam: Can't really read that. Newb-light is target.

davisjam · 2018-03-04T22:19:36Z

@joshbruce Acknowledged.

These are based on test/original/inline_html_simple.md

davisjam · 2018-03-04T22:21:12Z

@joshbruce Is the code in b681158 more readable?

joshbruce · 2018-03-04T22:22:54Z

@davisjam: Try one test per example??

See also: #1124 (comment)

davisjam · 2018-03-04T22:24:18Z

@joshbruce Hesitant to do that since there would be a ton of boilerplate. Thoughts?

joshbruce · 2018-03-04T22:41:09Z

@davisjam: I might need to slow down a bit in response time...really jazzed to see all the cooperation and collaboration...y'all are awesome!

...question:

Define "boilerplate" in this context?

Considerations (imho):

Each test should be discreet (seem to remember there a mnemonic here STAR?? I think, COMET...I don't know): it('should pass CM 123' - this should help isolate which specific test scenario is failing without being the general heading "inline elements" and should be easier to migrate should the spec ever change...added new example -> add new test...changed specific example -> update.
If we get an issue with a sample of MD and HTML - we can write it('should pass issue 1234 - to protect against regression and make triaging fixes easier.
The size of the test bed shouldn't matter, if that's the concern related to boilerplate. It's mainly there to act as a technical specification for users to see how to use marked and get an idea of what to expect. If the test is in there, we say we can do it; if it's not...well. (Maybe we would do something like it('should fail CM 789' to let people know, "We don't support that example" (see the emphasis problem).

Having said that, I'm not a big fan of writing tests for their own sake...the fact we're doing this against a spec makes it a little more awkward than what I was doing there at the moment...partially, I think because our renderer is slightly complex compared to the type definition of an HTML element per #1102 (comment) - which is what I ended up with after starting with a PHP associative array (JSON). It makes the NPM package size bigger, but that's not what people are using operationally in their sites...if I have a question on why something is happening; instead of running to GitHub, I can run to the tests or spec (make sure I'm suing the right combination, make sure it's supported, then maybe post a PR to make the test pass).

Something like that...??

davisjam · 2018-03-04T23:12:03Z

Each test should be discreet

Yes. I'm writing tests at the level of "marked honors inline html" or "marked honors inline links". Spec-by-spec.

These are based on test/original/auto_links.md

davisjam · 2018-03-04T23:43:51Z

Okie dokie, I'll leave this for now.

@joshbruce @UziTech Let me know how the current version looks.

These are based on test/original/amps_and_angles_encoding.md

davisjam · 2018-03-05T02:10:42Z

Currently 'npm run lint' does not scan the new tests.

"lint": "eslint --fix lib/marked.js test/index.js",

Can we just replace "test/index.js" with "test/"? Seems to work.

UziTech · 2018-03-05T06:06:21Z

@davisjam really we should add an .eslintignore file for marked.min.js (I'm pretty sure that is the only files we want to ignore) and use eslint --fix .

UziTech · 2018-03-05T06:13:41Z

test/integration/marked-spec.js

+                    ];
+
+      samples.forEach(function(sample) {
+        expect(marked(sample.md)).toBe(sample.html);


We should figure out how to get a better error message other than expected 'some really long string' to equal 'some other really long string'.

We could try something like jasmine-diff-matchers

UziTech · 2018-03-05T06:14:08Z

package.json

@@ -38,7 +38,7 @@
    "uglify-js": "^3.3.10"
  },
  "scripts": {
-    "test": "jasmine --config=jasmine.json",
+    "test": "node test && jasmine --config=jasmine.json",


test/specs/specs-spec.js already takes care of running the test/index.js tests in jasmine so there is no need to run node test.

UziTech · 2018-03-08T21:09:23Z

test/integration/marked-spec.js

+                     {'md': 'Inline [link](/script?foo=1&bar=2) with ampersands', 'html': '<p>Inline <a href="/script?foo=1&amp;bar=2">link</a> with ampersands</p>\n'},
+                     {'md': '[link with ampersands](http://example.com/?foo=1&bar=)', 'html': '<p><a href="http://example.com/?foo=1&amp;bar=">link with ampersands</a></p>\n'},
+                     {'md': '[link title with ampersands](http://att.com "AT&T")', 'html': '<p><a href="http://att.com" title="AT&amp;T">link title with ampersands</a></p>\n'},
+                    ];


I think it would be easier to just pull these from their current files. I hate hard coding strings in javascript. Too much possibility for errors.

We could construct each test from the files and add the describe and it descriptions to the front-matter in the .md file

or maybe reorganize these tests and pull the describe description from the folder name and the it description from the file name

joshbruce · 2018-03-24T13:56:56Z

See #1159

UziTech · 2018-04-04T18:35:22Z

I think we should try to converge on a similar test pattern as #1160, adding a json file for the original test similar to commonmark.json

joshbruce · 2018-04-05T03:12:51Z

I think if we're going to Jasminify the legacy tests, we might be covered elsewhere. #1160, for example, should/would cover the CM tests; so, they can be removed. The GFM ones would be covered by the GFM tests that haven't been created yet. There may also be conflicts between the specs and what marked does (autolinks, for example, might include a mechanism not covered by CommonMark).

Point being I suppose that should we keep these around just because they've always been around? or, can we rationally deprecate (some, most, many of) them in favor of something more robust?

UziTech · 2018-04-05T04:48:44Z

I like the idea of just having CommonMark and GFM tests and deprecating all other features in favor of making marked more extensible and less bloated.

We should probably keep the tests around until the options are removed (in 1.x or 2.x)

UziTech · 2019-03-12T18:26:42Z

I'm closing this in favor of #1444

davisjam requested a review from UziTech March 4, 2018 20:12

UziTech reviewed Mar 4, 2018

View reviewed changes

davisjam added 2 commits March 4, 2018 17:06

test: inline link tests

d74044c

These are based on test/original/links_inline_style.md

test: run old tests before new

a693f23

davisjam force-pushed the MoreJasmineTests branch from afab39f to a693f23 Compare March 4, 2018 22:06

test: inline html tests

b681158

These are based on test/original/inline_html_simple.md

davisjam force-pushed the MoreJasmineTests branch from 20d0ef7 to b681158 Compare March 4, 2018 22:20

rewrite with md -> html format

e86f097

davisjam force-pushed the MoreJasmineTests branch from 9d2779a to e86f097 Compare March 4, 2018 23:23

davisjam added 2 commits March 4, 2018 18:40

test: auto links

651e0c8

These are based on test/original/auto_links.md

test: comments

cde69ae

davisjam changed the title ~~test: inline link tests~~ WIP: test: jasmine-ify original tests Mar 4, 2018

test: special characters

6dd532e

These are based on test/original/amps_and_angles_encoding.md

UziTech requested changes Mar 5, 2018

View reviewed changes

Feder1co5oave mentioned this pull request Mar 8, 2018

Attempting CM 331-338 #1124

Closed

6 tasks

UziTech reviewed Mar 8, 2018

View reviewed changes

joshbruce mentioned this pull request Mar 24, 2018

Build CommonMark test bed #1159

Closed

UziTech closed this Mar 12, 2019

ekmixon mentioned this pull request Jul 8, 2022

[Snyk] Security upgrade rollup-plugin-license from 2.3.0 to 2.8.0 ekmixon/marked#100

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: test: jasmine-ify original tests #1129

WIP: test: jasmine-ify original tests #1129

davisjam commented Mar 4, 2018 •

edited

Loading

UziTech Mar 4, 2018

davisjam Mar 4, 2018

davisjam commented Mar 4, 2018

davisjam commented Mar 4, 2018

UziTech commented Mar 4, 2018

UziTech commented Mar 4, 2018

joshbruce commented Mar 4, 2018

davisjam commented Mar 4, 2018

davisjam commented Mar 4, 2018

joshbruce commented Mar 4, 2018

davisjam commented Mar 4, 2018

joshbruce commented Mar 4, 2018

davisjam commented Mar 4, 2018

davisjam commented Mar 4, 2018

davisjam commented Mar 5, 2018 •

edited

Loading

UziTech commented Mar 5, 2018 •

edited

Loading

UziTech Mar 5, 2018

UziTech Mar 5, 2018

UziTech Mar 8, 2018

UziTech Mar 8, 2018 •

edited

Loading

joshbruce commented Mar 24, 2018

UziTech commented Apr 4, 2018

joshbruce commented Apr 5, 2018 •

edited

Loading

UziTech commented Apr 5, 2018

UziTech commented Mar 12, 2019

WIP: test: jasmine-ify original tests #1129

WIP: test: jasmine-ify original tests #1129

Conversation

davisjam commented Mar 4, 2018 • edited Loading

UziTech Mar 4, 2018

Choose a reason for hiding this comment

davisjam Mar 4, 2018

Choose a reason for hiding this comment

davisjam commented Mar 4, 2018

davisjam commented Mar 4, 2018

UziTech commented Mar 4, 2018

UziTech commented Mar 4, 2018

joshbruce commented Mar 4, 2018

davisjam commented Mar 4, 2018

davisjam commented Mar 4, 2018

joshbruce commented Mar 4, 2018

davisjam commented Mar 4, 2018

joshbruce commented Mar 4, 2018

davisjam commented Mar 4, 2018

davisjam commented Mar 4, 2018

davisjam commented Mar 5, 2018 • edited Loading

UziTech commented Mar 5, 2018 • edited Loading

UziTech Mar 5, 2018

Choose a reason for hiding this comment

UziTech Mar 5, 2018

Choose a reason for hiding this comment

UziTech Mar 8, 2018

Choose a reason for hiding this comment

UziTech Mar 8, 2018 • edited Loading

Choose a reason for hiding this comment

joshbruce commented Mar 24, 2018

UziTech commented Apr 4, 2018

joshbruce commented Apr 5, 2018 • edited Loading

UziTech commented Apr 5, 2018

UziTech commented Mar 12, 2019

davisjam commented Mar 4, 2018 •

edited

Loading

davisjam commented Mar 5, 2018 •

edited

Loading

UziTech commented Mar 5, 2018 •

edited

Loading

UziTech Mar 8, 2018 •

edited

Loading

joshbruce commented Apr 5, 2018 •

edited

Loading