-
Notifications
You must be signed in to change notification settings - Fork 125
Add mutation testing with Stryker (and restructure some files) #413
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
markdown-link-check
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stryker cannot figure out what to do with this file because it has no extension.
I have moved the contents of this into a src
directory to follow a more conventional file structure and allow for breaking things into smaller bites along the lines of the single responsibility principal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming we are happy with the initial file re-org, I might move that out to a separate pull request that the resulting commits when we squash and merge cover one concern each.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another reason for having the file extension mentioned in #415.
@@ -5,7 +5,7 @@ | |||
"bin": { | |||
"markdown-link-check": "markdown-link-check" | |||
}, | |||
"main": "index.js", | |||
"main": "markdown-link-check", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is correct 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tcort @smainil @BaseMax It would be good to get some other brains/eyes on this.
index.js
(moved in this pull request to src/index.js
) seems to contain things used in src/markdown-link-check.js
and should not be used directly.
I think this change is correct, but maybe there is something I'm missing?
@WillGibson great job. Looks good to me. In addition to unit test and coverage, I think we need to be able to run integration test invoking the check through command line, docker or directly from a nodejs source code PS: sorry if my english is not as good as yours. It's not my native language |
Good idea. Something like this in a GitHub, but against a file we expect to have only alive links?
That would check it all hooks up correctly and we can leave the detail to the unit tests. Anything other than a zero exit code would be a fail. I'll knock up a pull request now...
I honestly wouldn't have noticed if you hadn't mentioned it :-) |
This pull request has been marked as stale because it has been open 60 days with no activity. It will be closed in 30 days unless the stale label is removed or someone adds a comment. |
@@ -5,7 +5,7 @@ | |||
"bin": { | |||
"markdown-link-check": "markdown-link-check" | |||
}, | |||
"main": "index.js", | |||
"main": "markdown-link-check", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tcort @smainil @BaseMax It would be good to get some other brains/eyes on this.
index.js
(moved in this pull request to src/index.js
) seems to contain things used in src/markdown-link-check.js
and should not be used directly.
I think this change is correct, but maybe there is something I'm missing?
@@ -2,326 +2,8 @@ | |||
|
|||
'use strict'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mutation testing could not deal with this file not having a .js
extension, so I moved it to src/markdown-link-check.js
and this file just executes that code now.
Also removed some unused variables etc. in the new file now that it's being checked by the linter because it has a .js
extension.
The intention is to help identify areas of the code not covered by tests so that we can start to remediate that and increase confidence in releases.
This is the state of play right now...