Revert "fs: validate args of truncate functions in js"

[MylesBorins]

Checklist

• make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

fs

Description of change

This reverts commit c86c1ee.

original commit message:

This patch

 1. moves the basic validation of arguments to `truncate` family
    of functions to the JavaScript layer from the C++ layer.

 2. makes sure that the File Descriptors are validated strictly.

PR-URL: #2498
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

This commit needs to be reverted in order to revert 9359de9

Please see more about the discussion in #7846

[MylesBorins]

CI: https://ci.nodejs.org/job/node-test-pull-request/3511/

[misterdjules]

LGTM.

[rvagg]

lgtm

[JungMinu]

LGTM

[jasnell]

LGTM

[MylesBorins]

landed in c5a18e7

[ChALkeR]

Ah, so we ended up reverting the tests? Could those be re-applied later, perhaps?

Refs: #7846 (comment)
Refs: #7899 (comment)

[jasnell]

Likely best to just open a new PR adding those back in. As a reminder for myself: were those going to be kept as known-issue tests or regular tests?

[ChALkeR]

@jasnell My understanding was that (sub)tests that are still passing should be kept as regular tests.

E.g. assert.throws(function() { fs.truncateSync([]); }, /path must be a string/); passes even on v6.3.1.

[MylesBorins]

@ChALkeR would you be willing to submit another PR with the appropriate tests included built on top of this PR? I'm on vacation and don't have a ton of time to invest ATM, but would like to see this land

[cjihrig]

@thealphanerd this doesn't apply cleanly to v6.x. Could you please backport it.

[cjihrig]

Actually, this reverts PR #2498, which was also labeled as semver major. That shouldn't have gone out in any releases. Did it? It doesn't look like it did. If it didn't, this should have the don't land on v6.x label.

@jasnell any info here?

[jasnell]

It shouldn't have. I believe this revert was necessary to do the other reverts. If we get any more of these issues with fs I'm going to curl up in the corner and cry for a bit.

[cjihrig]

I don't believe this needs to be backported to v6, so I'm going to add the label. Somebody (anybody) correct me if that is incorrect.

[MylesBorins]

@cjihrig this should not have to land on v6.x for the above mentioned reasons. I'll make sure to get the label on there next time