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

Added support to paste flows exported from editor #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Using the test-helper, your tests can start the Node-RED runtime, load a flow an

## Adding to your node project dependencies

To add unit tests your node project test dependencies, add this test helper as follows:
To add unit tests to your node project test dependencies, add this test helper as follows:

npm install node-red-node-test-helper --save-dev

Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


Instead of installing the unit test node project test dependencies, which can pull in a very large number of packages, you can install the unit test packages globally and link them to your node project. This is a better option if you plan on developing more than one node project.

Install the unit test packages globally as follows:

npm install -g node-red
npm install -g node-red-node-test-helper
npm install -g should
npm install -g mocha
npm install -g sinon
npm install -g supertest
npm install -g express

In your node project development directory, link the unit test packages as follows:

npm link node-red
npm link node-red-node-test-helper
npm link should
npm link mocha
npm link sinon
npm link supertest
npm link express

Depending on the nodes in your test flow, you may also need to link in other packages as required. If a test indicates that a package cannot be found, install the package globally and then link it to your node project the same way as the packages above.

## Adding test script to `package.json`

To run your tests you can add a test script to your `package.json` file in the `scripts` section. To run all of the files with the `_spec.js` prefix in the test directory for example:
Expand Down Expand Up @@ -88,6 +114,14 @@ In this example, we require `should` for assertions, this helper module, as well

We then have a set of mocha unit tests. These tests check that the node loads correctly, and ensures it makes the payload string lower case as expected.

## Creating test flows in the Node Red editor

The Node Red editor can be used to generate test flow configurations. Create a flow in the editor with the node you wish to test and configure them using the node configuration editor. Add `debug` nodes to receive output messages from your test node. `catch` and `status` nodes can also be used to catch errors and status changes from your test node. It is not necessary to include `inject` nodes as this helper module will allow you to inject test messages.

Highlight the nodes in the test flow and select `Export` then `Clipboard` to copy your test flow configuration, and then paste the JSON string into the test script. Repeat this process to create different variations of your test flow if required.

When the flow is run in this helper module, `debug` nodes are converted to `helper` nodes.

## Getting nodes in the runtime

The asynchronous `helper.load()` method calls the supplied callback function once the Node-RED server and runtime is ready. We can then call the `helper.getNode(id)` method to get a reference to nodes in the runtime. For more information on these methods see the API section below.
Expand Down Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

* testCredentials: (object) Optional node credentials.
* cb: (function) Function to call back when testFlows has been started.

Expand Down Expand Up @@ -194,4 +228,4 @@ var logEvents = helper.log().args.filter(function(evt {

npm run test

This runs tests on snaphots of some of the core nodes' Javascript files to ensure the helper is working as expected.
This runs tests on snaphots of some of the core nodes' Javascript files to ensure the helper is working as expected.
27 changes: 24 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ var events = require('node-red/red/runtime/events');

var app = express();

var util = require('util');

var address = '127.0.0.1';
var listenPort = 0; // use ephemeral port
var port;
Expand All @@ -41,10 +43,14 @@ var server;

function helperNode(n) {
RED.nodes.createNode(this, n);

this.error = function(logMessage,msg) {
console.log(logMessage);
}
}

module.exports = {
load: function(testNode, testFlows, testCredentials, cb) {
load: function(testNode, testFlow, testCredentials, cb) {
var i;

logSpy = sinon.spy(log,"log");
Expand All @@ -61,9 +67,24 @@ module.exports = {
testCredentials = {};
}

if (typeof testFlow === "string") {
testFlow = JSON.parse(testFlow);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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().

}

var flowId = testFlow[0].z || "f1";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@mblackstock mblackstock Feb 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

node.type ="helper";
}
});

testFlow.unshift({id: flowId, type:"tab", label:"Test flow"});

var storage = {
getFlows: function() {
return when.resolve({flows:testFlows,credentials:testCredentials});
return when.resolve({flows:testFlow,credentials:testCredentials});
}
};

Expand Down Expand Up @@ -94,7 +115,7 @@ module.exports = {
}
flows.load().then(function() {
flows.startFlows();
should.deepEqual(testFlows, flows.getFlows().flows);
should.deepEqual(testFlow, flows.getFlows().flows);
cb();
});
},
Expand Down
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
},
{
"name": "Mike Blackstock"
},
{
"name": "Dean Cording"
}
],
"keywords": [
Expand Down
4 changes: 4 additions & 0 deletions test/function_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ describe('function node', function() {
helper.unload();
});

after(function(done) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

helper.stopServer(done);
});

it('should be loaded', function(done) {
var flow = [{id:"n1", type:"function", name: "function" }];
helper.load(functionNode, flow, function() {
Expand Down
1 change: 1 addition & 0 deletions test/httprequest_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ describe('HTTP Request Node', function() {

after(function() {
testServer.close();
helper.stopServer();
});
afterEach(function() {
helper.unload();
Expand Down
23 changes: 15 additions & 8 deletions test/lower-case_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,29 @@ describe('lower-case Node', function () {
});

it('should be loaded', function (done) {
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":[[]]}]';
Copy link
Contributor

@mblackstock mblackstock Feb 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


helper.load(lowerNode, flow, function () {
var n1 = helper.getNode("n1");
var n1 = helper.getNode("3912a37a.c3818c");
n1.should.have.property('name', 'lower-case');
done();
});
});

it('should make payload lower case', function (done) {
var flow = [
{ id: "n1", type: "lower-case", name: "test name",wires:[["n2"]] },
{ id: "n2", type: "helper" }
];

// Exported flow pasted as Javascript
var flow = [{"id":"3912a37a.c3818c","type":"lower-case","z":"e316ac4b.c85a2",
"name":"lower-case","x":240,"y":320,"wires":[["7b57d83e.378fd8"]]},
{"id":"7b57d83e.378fd8","type":"debug","z":"e316ac4b.c85a2","name":"",
"active":true,"tosidebar":true,"console":false,"tostatus":false,
"complete":"true","x":400,"y":340,"wires":[]}];

helper.load(lowerNode, flow, function () {
var n2 = helper.getNode("n2");
var n1 = helper.getNode("n1");
var n2 = helper.getNode("7b57d83e.378fd8");
var n1 = helper.getNode("3912a37a.c3818c");
n2.on("input", function (msg) {
msg.should.have.property('payload', 'uppercase');
done();
Expand Down
4 changes: 4 additions & 0 deletions test/websocket_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ describe('websocket Node', function() {
helper.unload();
});

after(function(done) {
helper.stopServer(done);
});

describe('websocket-listener', function() {
it('should load', function(done) {
var flow = [{ id: "n1", type: "websocket-listener", path: "/ws" }];
Expand Down