Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node test config:check always returns 0 #1928

Closed
doctorfree opened this issue Feb 11, 2020 · 3 comments
Closed

Node test config:check always returns 0 #1928

doctorfree opened this issue Feb 11, 2020 · 3 comments
Labels

Comments

@doctorfree
Copy link

Platform: electron@6.0.12, Raspberry Pi 4, Linux raspberrypi 4.19.97-v7l+

Node Version: v10.19.0

MagicMirror Version: 2.10.1

Description: The command "npm run config:check" always returns success regardless of the validity of the configuration file it is checking. The test is in the file tests/configs/check_config.js

Steps to Reproduce: Create an invalid config/config.js (e.g. delete the closing ] or } for a module) and run "npm run config:check" from a shell command prompt. After the npm command completes, run the command "echo $?". The output of "echo $?" is always 0 no matter which config file I check.

Expected Results: Checking valid configuration files should return 0 while checking invalid configurations should return 1 or other non-zero values.

Actual Results: The config:check test always returns 0

Configuration: I used a very simple config.js with the closing ] commented out:
modules: [
{
module: "alert",
},
{
module: "updatenotification",
position: "top_bar"
},
{
module: 'MMM-Remote-Control',
},
// ]

Additional Notes: This is a minor bug and would not effect most users. I am developing a command line script to manage my MagicMirror's many configuration files. One feature I have implemented is checking the validity of a configuration file before installing it. In order to do this programmatically in my script I needed to make the following one line change to the file tests/configs/check_config.js

*** check_config.js.orig 2020-02-10 14:29:40.399261804 -0800
--- check_config.js 2020-02-10 14:26:04.376118867 -0800


*** 61,66 ****
--- 61,67 ----
error = errors[idx];
console.log("Line", error.line, "col", error.character, error.reason);
}

  •       process.exit(1);
      }
    
    });
    }

Adding this call to process.exit(1) works for me but it may not be the best solution. I also found it cleaner, especially after this change, to run the config test with --silent option to npm:
npm run --silent config:check

My MagicMirror control script is in my Gitlab repository at https://gitlab.com/doctorfree/Scripts in the MagicMirror subdirectory at MagicMirror/mirror.sh

@stale
Copy link

stale bot commented Apr 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Apr 11, 2020
@fewieden
Copy link
Contributor

@doctorfree adding it in only this spot would not be correct. You also would add it here:

Nevertheless, I would rather go with https://eslint.org/docs/rules/no-process-exit recommendations and throw an error instead.

@stale stale bot removed the wontfix label Apr 11, 2020
@stale
Copy link

stale bot commented Jun 10, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants