Removing the use of eval in Node.js

[itayperry]

Purpose

Not allowing the use of eval in Node.js - as decided in this discussion: https://github.com/sinonjs/fake-timers/issues/319#issuecomment-632794814

In this pull request using an eval() in Node.js env will throw an error. At first, I've seen that there are 4 tests that test things who do not require eval() at all (thinking about https://github.com/sinonjs/fake-timers/pull/327#discussion_r429342307), I've replaced the strings in these tests with empty functions.

In addition, I've added 2 describe() test blocks to test the use of eval (instead of the previous eval tests - who ignored the environments), once in Node.js and once in other environments.

I've added a global variable that will only exist in a Node.js environment

[codecov[bot]]

Codecov Report

Merging #331 (012f4f2) into master (43dc8db) will decrease coverage by 1.70%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #331      +/-   ##
==========================================
- Coverage   93.98%   92.28%   -1.71%     
==========================================
  Files           1        1              
  Lines         549      557       +8     
==========================================
- Hits          516      514       -2     
- Misses         33       43      +10     

Flag	Coverage Δ
unit	92.28% <100.00%> (-1.71%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/fake-timers-src.js	92.28% <100.00%> (-1.71%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 43dc8db...32022bf. Read the comment docs.

[itayperry]

Hi, I rebased and the conflict that appeared at the beginning is now gone :)

[itayperry]

Cool :) I have one more thing to do regarding the eval() - to emit a warning, maybe show a console message, when the eval is used (not in Node.js)

[benjamingr]

This isn't merged yet because it's semver-major

[fatso83]

@benjamingr Not sure why that is a reason preventing merging? Just bump the version and release, or are you waiting to have this automated?

[benjamingr]

@fatso83 I didn't know when a release would happen and didn't want to release a major without talking to anyone mostly

[fatso83]

Oh! That's very considerate, but a release happens when people have time to do it. If we need to coordinate something it's usually enough to tag the maintainer team. You and Simen are the most active these days on this project, so you are probably best placed to make a decision on when to do it. @mroderick is also more active than me, so I should not be a blocker. We split out these subprojects to let them be maintained by people with the most interest in them after all, so that we could focus on the main Sinon project.

Basically, it should not matter. Nothing breaks if you do a semver upgrade.

[mroderick]

@benjamingr if you share your username on the npm registry with me, then I can add you to the sinonjs organisation, and you can publish the package yourself

[benjamingr]

https://www.npmjs.com/~benjamin.gruenbaum @mroderick

[benjamingr]

It also might make sense from a security PoV to remove peer5 (my old employer from when I started contributing which I believe was added) unless they've made commits since.

[mroderick]

@benjamingr I've sent an invite to the username and have removed peer5

[itayperry]

Is this PR ok? Is there anything I need to fix or something?

[mroderick]

Something seems to have gone wrong with updating this from master, as isNearInfiniteLimit is undefined. The tests fail because of out of heap memory.

[itayperry]

I'm sorry for the mess, I'm having a hard time working with GitHub's commands and branches sometimes. @fatso83 would you like to try merging it again? The isNearInfiniteLimit is now gone and all tests pass as before.

[fatso83]

This seems fine now, but as you can see from the status indicators on the individual commits, it still has a non-passing state. It is because of you not running Prettier. Just running npm run prettier:write should fix you up 😄

[fatso83]

You know what, I'll just fix it :)

[fatso83]

Thanks, this has now been merged.

[itayperry]

Awesome! Thank you! :)