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

Update majority of original and new test beds #1229

Closed
wants to merge 73 commits into from
Closed

Update majority of original and new test beds #1229

wants to merge 73 commits into from

Conversation

joshbruce
Copy link
Member

@joshbruce joshbruce commented Apr 17, 2018

Marked version: 0.3.19

Markdown flavor: Markdown.pl|CommonMark|GitHub Flavored Markdown|n/a

Description

  • Deprecates test cases from original test suite that are covered by CommonMark and GFM test suites.
  • Migrates previous scenarios from the original test suite, where applicable.
  • Adds verifiable "Gruber" flavor where possible (original Perl).
// GFM
{
  pedantic: false,
  gfm: true
}

// CommonMark
{
  pedantic: false,
  gfm: false,
  xhtml: true,
  headerIds: false
}

// Gruber
{
  pedantic: true,
  gfm: false
}

// Issues/PRs
// For tests born from an issue or a PR, the example number will be set to the issue or PR number

Need help understanding why test exists

  • /new/double_link

Contributor

  • Test(s) exist to ensure functionality and minimize regression (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.

Committer

In most cases, this should be a different person than the contributor.

  • Draft GitHub release notes have been updated.
  • CI is green (no forced merge required).
  • Merge PR

@joshbruce
Copy link
Member Author

Well Snyk, WIP, and review guidelines seem to work. :)

@joshbruce
Copy link
Member Author

This is a lot of change. I've converted and eliminated what I think is fair. Thinking we should merge this with the understanding that there will be some we might need to do differently (bringing in the full pages from Gruber) or that I need a second set of eyes on (the redos).

@joshbruce joshbruce changed the title [WIP]: Update test beds with Gruber and original set Update majority of original and new test beds Apr 21, 2018
});
});

describe('CommonMark 0.28 Fenced code blocks', function() {
var section = 'Fenced code blocks';

// var shouldPassButFails = [];
var shouldPassButFails = [93, 95, 96, 97, 101, 102, 106, 108, 111, 112, 113];
var shouldPassButFails = [88, 89, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 104, 105, 106, 108, 109, 110, 111, 112, 113, 115];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are more tests failing than before?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See testOptions. Before I turned headerIds off xhtml on...this time, I turned pedantic off, turned gfm off, headerIds off, xhtml on, and changed the value for langPrefix to match the one from the specification.

Marked doesn't really have a "CommonMark" setting or option. Those were the combination of settings that I thought would us close enough.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least that's my best guess.

describe('GFM 0.28 Tables', function() {
var section = 'Tables';

// TODO: Verify exmaple 193 is valid and passing
// var shouldPassButFails = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this comment

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...see last reply...Someone can uncomment that one, comment out the real one, and be set to go.

describe('Marked Issues & PRs', function() {
var section = 'Blockquotes';

// var shouldPassButFails = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused comment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...see last reply...Someone can uncomment that one, comment out the real one, and be set to go.

describe('Marked Issues & PRs', function() {
var section = 'Strikethrough';

// var shouldPassButFails = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused comment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...see last reply...Someone can uncomment that one, comment out the real one, and be set to go.


MarkedTester.test = function(spec, section, ignore, options) {
if (spec.section === section) {
var shouldFail = ~ignore.indexOf(spec.example);
Copy link
Member

@styfle styfle Apr 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered moving the logic for ignore into this test() function so that each test doesn't need to concat itself (should be less prone to error).

  MarkedTester.test = function(spec, section, options, shouldPassButFails, willNotBeAttemptedByCoreTeam) {
 
  var ignore = (shouldPassButFails || []).concat(willNotBeAttemptedByCoreTeam || []);
  // ...
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way, the last two parameters could be optional and make each test a little smaller.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting point and speaks a little to the "remove unused comments" reviews as well.

I'm trying to consider this as a long-term tool for others. So, each test suite is a self-contained way for someone to look and know what's going on...and easily get down to work...hence the commented out empty arrays. Someone can uncomment that one, comment out the real one, and be set to go.

Definitely, considering moving the concat step to the test though. Trying to focus on 8fold over the weekend though.

@UziTech
Copy link
Member

UziTech commented Mar 12, 2019

I'm closing this in favor of #1444

@UziTech UziTech closed this Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants