Skip to content

test: remove obsolete TODO comments for fs.readline()

Rodrigo Muino Tomonari requested to merge github/fork/Trott/readfile into master

The readfile/pipe tests rely on a pre-existing pipe in the system (namely, /dev/stdin). This does not exist on Windows.

There doesn't seem to be an easy way (or perhaps any way?) to do all of:

  • create a named pipe on Windows that...
  • ...can be hooked up to STDIN...
  • ...from inside Node.js

I tried a few creative-ish things (including abusing net.createServer().listen() to trick Node into creating a named pipe and then trying to get cmd.exe to write to that pipe), but no dice.

Here's the commit where this comment was initially introduced, if that helps: https://github.com/nodejs/io.js/commit/1d3d02c70dffb29e9c80a35fe9f791b357a1dccf

In the current code base, the line in lib.fs that is being changed is now https://github.com/nodejs/io.js/blob/5d2b846d1114bad5e7fbf04910ff59fc970c175e/lib/fs.js#L288

Changing -1 to pos in that line does not result in the corresponding test failing, so it's not clear that the test is addressing the change in that commit anymore (if it ever did?).

There is no reason to expect Windows to ever have this functionality.

So, in the interest of weed-whacking out all the TODOs in the tests, these should seemingly be removed. If there's consensus that this is wrong and it actually does capture useful information, perhaps it can be moved to the issue tracker rather than lurking in a code TODO.

Apologies in advance for another in a tedious series of TODO-removal PRs...

Merge request reports