Skip to content

stringify mockWrapper so we can generate coverage#243

Merged
ritch merged 1 commit into
masterfrom
fix-ci
Sep 16, 2015
Merged

stringify mockWrapper so we can generate coverage#243
ritch merged 1 commit into
masterfrom
fix-ci

Conversation

@rmg

@rmg rmg commented Sep 16, 2015

Copy link
Copy Markdown
Member

This has to be done because when we run the tests with coverage the
function body gets mangled by instrumentation and the string
replacements fail horribly. The end result is the generated function
ends up not even being valid JavaScript and the tests then fail for
obvious reasons.

This has to be done because when we run the tests with coverage the
function body gets mangled by instrumentation and the string
replacements fail horribly. The end result is the generated function
ends up not even being valid JavaScript and the tests then fail for
obvious reasons.
@rmg rmg added the #review label Sep 16, 2015
@ritch

ritch commented Sep 16, 2015

Copy link
Copy Markdown
Member

LGTM

ritch added a commit that referenced this pull request Sep 16, 2015
stringify mockWrapper so we can generate coverage
@ritch
ritch merged commit ad514ea into master Sep 16, 2015
@ritch ritch removed the #review label Sep 16, 2015
@ritch
ritch deleted the fix-ci branch September 16, 2015 06:18
@bajtos

bajtos commented Sep 16, 2015

Copy link
Copy Markdown
Member

Thanks @rmg for fixing this issue.

I wish we could keep using toString, so that the function body can be run through the linter and we can use IDE auto-completion while changing its code, but I understand that's not feasible with our CI setup.

@rmg

rmg commented Sep 16, 2015

Copy link
Copy Markdown
Member Author

@bajtos there are some alternatives that would allow the function to remain a function:

  • extract the function to a separate file and exclude that file from coverage instrumentation
  • disable code coverage for the entire project

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