After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 703826 - system.exit() expects conflicting number of arguments
system.exit() expects conflicting number of arguments
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other All
: Normal major
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2013-07-08 21:28 UTC by Philip Chimento
Modified: 2013-08-06 10:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tests: Rename installed-tests/test to installed-tests/extra (982 bytes, patch)
2013-07-26 00:27 UTC, Colin Walters
none Details | Review
modules/system: Fix system.exit (1.12 KB, patch)
2013-07-26 00:27 UTC, Colin Walters
accepted-commit_now Details | Review
tests: Add new installed-tests/scripts test mechanism (2.07 KB, patch)
2013-07-26 00:27 UTC, Colin Walters
none Details | Review
tests: Add new installed-tests/scripts test mechanism (2.10 KB, patch)
2013-07-26 18:42 UTC, Colin Walters
none Details | Review

Description Philip Chimento 2013-07-08 21:28:58 UTC
const System = imports.system;
System.exit(0);

produces

** (gjs:19889): CRITICAL **: gjs_parse_args: assertion `arg_location != NULL' failed
JS_EvaluateScript() failed but no exception message?

while 

const System = imports.system;
System.exit();

produces

    JS ERROR: !!!   Exception was: Error: Error invoking exit: Expected 1 arguments, got 0
    JS ERROR: !!!     message = '"Error invoking exit: Expected 1 arguments, got 0"'
    JS ERROR: !!!     fileName = '"main.js"'
    JS ERROR: !!!     lineNumber = '17'
    JS ERROR: !!!     stack = '"@main.js:17

I think either the number of args should be 1 in the JS_DefineFunction() call on l.158 of system.c (https://git.gnome.org/browse/gjs/tree/modules/system.c#n158) or gjs_exit() shouldn't call gjs_parse_args() on l.116 (https://git.gnome.org/browse/gjs/tree/modules/system.c#n116).

I don't know what your desired API for system.exit() is, so I didn't prepare a patch, but either way it's a one or two-line fix.

Personally, the API with the exit code argument makes more sense to me.
Comment 1 Colin Walters 2013-07-26 00:27:34 UTC
Created attachment 250154 [details] [review]
tests: Rename installed-tests/test to installed-tests/extra

Since really the directory is extra stuff.
Comment 2 Colin Walters 2013-07-26 00:27:42 UTC
Created attachment 250155 [details] [review]
modules/system: Fix system.exit

See patch.
Comment 3 Colin Walters 2013-07-26 00:27:46 UTC
Created attachment 250156 [details] [review]
tests: Add new installed-tests/scripts test mechanism

The unit tests are not suitable for all sorts of tests; in particular,
I can't test System.exit() from there since it loads all of the
tests into one process.

So this patch adds a new type of "scripts" test which is literally
just a gjs script; if it exits successfully, the test passed.
Comment 4 Jasper St. Pierre (not reading bugmail) 2013-07-26 01:54:40 UTC
Review of attachment 250155 [details] [review]:

OK.
Comment 5 Colin Walters 2013-07-26 18:42:00 UTC
Created attachment 250224 [details] [review]
tests: Add new installed-tests/scripts test mechanism

Throw an explicit error if we fail to exit
Comment 6 Colin Walters 2013-08-06 10:06:04 UTC
Pretty confident in the small tests changes, we can always improve them later.