Add support for --preload-file and under node
#11785
Conversation
I did this to all me to test the pre-load plugins which don't run with `--embed-file`. Is it worth landing this for testing reasons alone. Shoudl the preload plugins work under node?
|
Is this useful? |
|
I think it's useful in that it removes a difference between web and non-web. |
|
Ok I'll try to get his landded then since it seems it might be useful outside of testing too. |
|
Is there any chance that we can merge this change recently? It's quite useful feature. |
|
I am very much in favor of landing this. @sbc100 if you don't have time for it, I can pick it up? |
|
I think this probably just needs a test. I don't have time to look at it today so if do please go ahead! |
| self.assertContained('|hello from a file wi|', self.run_js('a.out.js')) | ||
| self.run_process([EMXX, 'main.cpp'] + args) | ||
| # run in node.js to ensure we verify that file preloading works there | ||
| result = self.run_js('a.out.js', engine=config.NODE_JS) |
There was a problem hiding this comment.
I don't think you need engine=config.NODE_JS here.. its the default for all tests in this file.
There was a problem hiding this comment.
This is defending against the default possibly changing in the future. Seems safer to make it explicit in places where it is specifically needed?
There was a problem hiding this comment.
OK, I think we should come up with a better way to specify node-only tests in the future. Maybe @requires_node or something like that.
tools/file_packager.py
Outdated
| errback(err); | ||
| } else { | ||
| console.log('got data'); | ||
| console.log(typeof contents.buffer); |
There was a problem hiding this comment.
Remove these two lines.
|
As your own PR I don't think you can approve it @sbc100 but does it look good now after the last changes? |
| @@ -712,6 +710,17 @@ def was_seen(name): | |||
|
|
|||
| ret += r''' | |||
| function fetchRemotePackage(packageName, packageSize, callback, errback) { | |||
| if (typeof process === 'object') { | |||
| fs = require('fs'); | |||
I did this to help me to test the pre-load plugins which don't run
with
--embed-file. Is it worth landing this for testing reasonsalone. Should the preload plugins work under node?