Added support to paste flows exported from editor #2

DeanCording · 2018-02-26T15:16:33Z

Allows flows to be create in editor and pasted into test scripts as JSON string. Backwards compatible with existing test flow definitions.
Automatically creates a flow to hold the test nodes so that Catch and Status nodes work.
Automatically converts Debug nodes into Helper nodes so that links can be set up between nodes using the editor.
Adds error handler to override node error handler so that test failures are passed back to mocha and not swallowed by Node Red
Fixes some existing tests that fail the stop the help server after tests are completed.
Documentation added for linking dependencies instead of installing them.

…reate a flow to contain them.

DeanCording · 2018-02-26T15:50:42Z

Currently the test helper doesn't use the Node Red registry to load the core nodes or other installed nodes. As a result it uses a deprecated interface to initialise nodes and requires the nodes in the test flow to be explicitly requireed in the test script.

I'm not sure how far to go with this. The original test harness started the bare minimum of Node Red to be able to run a node. This PR starts a bit more of Node Red to support a flow to contain the nodes being tested. Starting the registry effectively starts 90% of Node Red, at which point we might as well start the whole thing and embed it in the test module. The overhead of this isn't as bad as it sounds - you can start Node Red at the start of the test script and then add/remove flows for each test.

mblackstock · 2018-02-26T21:15:46Z

Thanks Dean. Ok. I'll have a look. Busy this week so might be slower to respond.

…

On Mon, Feb 26, 2018, 7:50 AM Dean Cording ***@***.***> wrote: Currently the test helper doesn't use the Node Red registry to load the core nodes or other installed nodes. As a result it uses a deprecated interface to initialise nodes and requires the nodes in the test flow to be explicitly requireed in the test script. I'm not sure how far to go with this. The original test harness started the bare minimum of Node Red to be able to run a node. This PR starts a bit more of Node Red to support a flow to contain the nodes being tested. Starting the registry effectively starts 90% of Node Red, at which point we might as well start the whole thing and embed it in the test module. The overhead of this isn't as bad as it sounds - you can start Node Red at the start of the test script and then add/remove flows for each test. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AArMeBusWT-1ChTaaY98qlcgRRhc_6WJks5tYtLTgaJpZM4STZmM> .

mblackstock · 2018-02-28T01:39:44Z

README.md

@@ -128,7 +162,7 @@ For additional test examples, see the `.js` files supplied in the `test/examples
 Loads a flow then starts the flow. This function has the following arguments:

 * testNode: (object|array of objects) Module object of a node to be tested returned by require function. This node will be registered, and can be used in testFlows.
-* testFlows: (array of objects) Flow data to test a node. If you want to use the flow data exported from Node-RED editor, need to covert it to JavaScript object using JSON.parse().
+* testFlows: (array of objects|JSON string)) Flow configuration to test a node. The flow configuration can be exported from Node-RED editor and pasted as a JSON string.


Why do we need to support a JSON string, can't we just paste from Node-RED as a JSON object into the unit test? No need to do a parse that way. JSON is valid Javascript after all (although more restrictive).

Yes, it is just covering all bases. I noted that in the README.md it did say that if you were exporting from the editor then you needed to covert it from JSON. In reality you don't need to as you can just paste the JSON in and it will work as Javascript.

Happy to take it out and clarify the documentation.

Thanks. I pulled that text blindly from the helper docs on the node-red wiki and didn't check it. :-(

mblackstock · 2018-02-28T01:40:23Z

index.js

+
+        testFlow.forEach(function(node) {
+            node.z = flowId;
+            if (node.type == "debug") {


Not sure we should be swapping node types. With this change, this module cannot ever be used to test the debug node, or another node called 'debug'. Do we want the core code to use this helper for consistency?

My thinking was that it would be good to be able to develop test flows in the editor and then export them into the test script without needing to change anything. As there is no helper node in the editor, the debug node is the logical choice as it would be used for debugging in the editor any way.

In any case, it turns out that the debug node won't load under test helper because parts of Node Red aren't properly initialised.

Hmm. The debug node tests run and pass in the core using the helper script there. will need to look at why this is happening.

Thinking it should be straightforward to search and replace 'debug' with 'helper'. We can add this to the docs on cutting and pasting the flows...or maybe provide a placeholder helper node with a UI as part of the framework?

The debug node tests definitely don't run under node-red-node-test-helper. The reason is that Node Red is only being partially initialised by the test helper and functions relating to interacting with the editor haven't been added to the RED object passed to the node when it is created. This problem will affect any other node that interacts with the editor, such as inject

The Debug node tests in core use the core test helper which is virtually identical to this one. So what specifically doesn't work? What errors do you get?

Running 58-debug_spec.js under the test helper gives:

Uncaught TypeError: Cannot set property message of AssertionError which has only a getter at test/58-debug_spec.js:88:21 at WebSocket.<anonymous> (test/58-debug_spec.js:605:13) at Receiver._receiver.onmessage (/usr/lib/node_modules/ws/lib/websocket.js:137:47) at Receiver.dataMessage (/usr/lib/node_modules/ws/lib/receiver.js:409:14) at perMessageDeflate.decompress (/usr/lib/node_modules/ws/lib/receiver.js:366:40) at _decompress (/usr/lib/node_modules/ws/lib/permessage-deflate.js:309:9) at _inflate.flush (/usr/lib/node_modules/ws/lib/permessage-deflate.js:395:7) at afterWrite (_stream_writable.js:454:3) at onwrite (_stream_writable.js:445:7) at InflateRaw.afterTransform (_stream_transform.js:90:3) at Zlib.callback (zlib.js:515:5)

Running any tests that include debug nodes give:

TypeError: Cannot read property 'post' of null at Array.module.exports (/usr/lib/node_modules/node-red/nodes/core/core/58-debug.js:223:19) at Object.load (index.js:112:28) at Context.<anonymous> (test/lower-case_spec.js:36:12)

From the stack trace it looks like you're attempting to test the debug node with the lower case test?

Looks like the core debug node tests call helper.startServer(), but lower-case tests don't.

The way to try it is to see if the core debug tests run with the module. When I copy the core debug node to the module examples (including the lib folder), and the debug test to the test helper project, the debug node tests pass for me.

mblackstock · 2018-02-28T01:42:47Z

test/lower-case_spec.js

-    var flow = [{ id: "n1", type: "lower-case", name: "lower-case" }];
+
+    // Exported flow pasted as JSON string
+    var flow = '[{"id":"3912a37a.c3818c","type":"lower-case","z":"e316ac4b.c85a2","name":"lower-case","x":240,"y":320,"wires":[[]]}]';


not sure why this change is needed. Just drop single quotes and it's equivalent to the original test as mentioned above re: JSON string. The original is easier to read imho - Id's are shorter, no unused wires.

I was using this as a test case for testing the export from the editor with debug nodes - that is why the id's are like that.

ok. how about an additional test called 'it should be loaded in exported flow'?

No, no - I was using lower-case as a test bed. The Id's and unused wires are what is exported from the editor.

mblackstock · 2018-02-28T01:45:17Z

index.js

+                testFlow = JSON.parse(testFlow);
+        }
+
+        var flowId = testFlow[0].z || "f1";


I like that the tab is included automatically. I will need to reverse my PR merge to master on my fork.

mblackstock · 2018-02-28T01:46:25Z

README.md

@@ -128,7 +162,7 @@ For additional test examples, see the `.js` files supplied in the `test/examples
 Loads a flow then starts the flow. This function has the following arguments:

 * testNode: (object|array of objects) Module object of a node to be tested returned by require function. This node will be registered, and can be used in testFlows.
-* testFlows: (array of objects) Flow data to test a node. If you want to use the flow data exported from Node-RED editor, need to covert it to JavaScript object using JSON.parse().
+* testFlows: (array of objects|JSON string)) Flow configuration to test a node. The flow configuration can be exported from Node-RED editor and pasted as a JSON string.


change name to testFlow in docs to match new method signature.

mblackstock · 2018-02-28T01:47:46Z

test/function_spec.js

@@ -29,6 +29,10 @@ describe('function node', function() {
        helper.unload();
    });

+    after(function(done) {


We should fix this in the core test code if it is causing problems

mblackstock

I suggest that we don't swap 'debug' nodes for 'helper' nodes. We might want to use this module in the core so we don't maintain two helper modules. Suggest we drop support for a string since it's not really necessary? Users can still cut and paste into their code, just don't use single quotes.

knolleary · 2018-02-28T21:22:51Z

My 2c - my preference would be to prioritise getting this to the point where we can run the tests we already have in core using this module rather than the helper bundled over there. Only then should we look at what other apis/features are potentially missing.

This test helper is demonstrably useful as-is - so lets stabilise that and get it documented and publicised.

mblackstock · 2018-02-28T21:41:30Z

OK - agreed

mblackstock · 2018-03-01T00:20:09Z

The best way to test that the helper will serve both core and contrib nodes will be to add the test helper module to the core as a dependency and use that code to run the core tests. I will work on that to ensure it works, and do a PR on the core. I suggest we set this PR aside for now and continue this conversation on slack? How about a new #testing channel?

boneskull · 2018-03-14T17:22:56Z

README.md

@@ -22,6 +22,32 @@ This will add the helper module to your `package.json` file as a development dep

 Both [Mocha](https://mochajs.org/) and [Should](https://shouldjs.github.io/) will be pulled in with the test helper.  Mocha is a unit test framework for Javascript; Should is an assertion library.  For more information on these frameworks, see their associated documentation.

+## Alternate linking of node project dependencies


I think we should avoid recommending this setup, as it's prone to weirdness depending on whatever version of npm is running. It's best practice to just add the dev dependencies, as this makes it easy to run the tests both in local development environments and CI (like Travis-CI). Linking is only really useful when you are actively developing a dependency.

Example: my node is node-red-contrib-foobar, and it needs dependency foobar.
But foobar has a bug affecting my NR Node, and I want to fix it. I clone foobar from GitHub, make my changes, run npm link in the working copy. Then, I return to my node-red-contrib-foobar working copy and run npm link foobar. My bug fix will then be reflected in node-red-contrib-foobar. If I make more changes to foobar, node-red-contrib-foobar automatically gets them, because of the link.

boneskull · 2018-03-14T17:25:56Z

index.js

@@ -61,9 +67,24 @@ module.exports = {
            testCredentials = {};
        }

+        if (typeof testFlow === "string") {
+                testFlow = JSON.parse(testFlow);


this will throw if testFlow is a string but not valid JSON. you will probably want to try / catch and pass the exception to cb().

natcl · 2021-03-09T15:28:43Z

Curious to see if there are plans ton include this ?
I'm looking for an easier way to test longer flows and it seems this PR could help.

Added support to paste flows exported from editor and automatically c…

8d6dc05

…reate a flow to contain them.

mblackstock reviewed Feb 28, 2018

View reviewed changes

boneskull reviewed Mar 14, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support to paste flows exported from editor #2

Added support to paste flows exported from editor #2

DeanCording commented Feb 26, 2018

DeanCording commented Feb 26, 2018

mblackstock commented Feb 26, 2018 via email

mblackstock Feb 28, 2018

DeanCording Feb 28, 2018

mblackstock Feb 28, 2018

mblackstock Feb 28, 2018

DeanCording Feb 28, 2018

mblackstock Feb 28, 2018 •

edited

DeanCording Feb 28, 2018

knolleary Feb 28, 2018

DeanCording Feb 28, 2018

mblackstock Mar 1, 2018

mblackstock Feb 28, 2018 •

edited

DeanCording Feb 28, 2018

mblackstock Feb 28, 2018

DeanCording Feb 28, 2018

mblackstock Feb 28, 2018

mblackstock Feb 28, 2018

DeanCording Feb 28, 2018

mblackstock Feb 28, 2018

DeanCording Feb 28, 2018

mblackstock left a comment

knolleary commented Feb 28, 2018

mblackstock commented Feb 28, 2018

mblackstock commented Mar 1, 2018

boneskull Mar 14, 2018

boneskull Mar 14, 2018

natcl commented Mar 9, 2021

		@@ -22,6 +22,32 @@ This will add the helper module to your `package.json` file as a development dep

		Both [Mocha](https://mochajs.org/) and [Should](https://shouldjs.github.io/) will be pulled in with the test helper. Mocha is a unit test framework for Javascript; Should is an assertion library. For more information on these frameworks, see their associated documentation.

		## Alternate linking of node project dependencies

Added support to paste flows exported from editor #2

Are you sure you want to change the base?

Added support to paste flows exported from editor #2

Conversation

DeanCording commented Feb 26, 2018

DeanCording commented Feb 26, 2018

mblackstock commented Feb 26, 2018 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mblackstock Feb 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mblackstock Feb 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mblackstock left a comment

Choose a reason for hiding this comment

knolleary commented Feb 28, 2018

mblackstock commented Feb 28, 2018

mblackstock commented Mar 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natcl commented Mar 9, 2021

mblackstock Feb 28, 2018 •

edited

mblackstock Feb 28, 2018 •

edited