arthurintelligence / node-fs-filesystem

NodeJS Filesystem Utility
MIT License
19 stars 2 forks source link

Linux: test fails when LANG is not english; parsing is weak on a new test case #15

Closed jacargentina closed 6 years ago

jacargentina commented 6 years ago

I've found that parsing currently depends on specific english strings; i'm from Argentina (spanish).

For example, npm run test with LANG=es_AR.utf8 makes test fail like this:

1) node-fs-filesystem
       linux integration tests
         native tests
           should properly parse the output of the filesystem command:
     TypeError: Cannot read property 'slice' of null

null is a failing match for the string /Disk\s(.*):\s.*,\s(\d+)\sbytes,\s(\d+) sectors/ on src/linux/linux.js:116

On spanish Disk is Disco

philippefutureboy commented 6 years ago

Great! So we should have a disclaimer on the README page that the library is locale/language dependent.

I think that in this case this is more of an enhancement than a bug, given that the library wasn't built with i18n in mind, and that we can't cover all languages programmatically.

We can manually extend the i18n coverage with something like the following:

  1. Add a i18n dictionary to the library for each key for each distro affected (linux and beyond)
  2. Replace the static regexes with new RegExp(...) with the keys dynamically fetched from the i18n dictionary

Let me know what you think,

Have a wonderful day ✨

jacargentina commented 6 years ago

@philippefutureboy I was thinking of another easier solution: we can force the environment for the child_process to have the LANG we need, ie LANG=en_US.utf8... using options.env, right?

philippefutureboy commented 6 years ago

I wasn't aware of this. If this is possible, it's marvelous :O

Is this possible across all computers or the user needs to have the English language pack installed?

jacargentina commented 6 years ago

Yeah, corelibs from linux distros are english by default; think macOS the same. No "language packs" needed.

jacargentina commented 6 years ago

I 've found on my work linux box that it fails to parse correctly; added my linux command output in a new file for testing static output; refactored the parsing to be stronger; now it works

philippefutureboy commented 6 years ago

Wonderful! Let me know how it goes for the MacOS side of things

jacargentina commented 6 years ago

I'm now trying to make parsing even better; if the user has no enough permissions, fdisk show errors, and then parsing is affected with strange errors like calling slice on an empty matches array.

jacargentina commented 6 years ago

Fix/4 is now included here.