-
Notifications
You must be signed in to change notification settings - Fork 42
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
|
@@ -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 | ||
|
||
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: | ||
|
@@ -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. | ||
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. :-( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change name to testFlow in docs to match new method signature. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
||
|
@@ -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. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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"); | ||
|
@@ -61,9 +67,24 @@ module.exports = { | |
testCredentials = {}; | ||
} | ||
|
||
if (typeof testFlow === "string") { | ||
testFlow = JSON.parse(testFlow); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this will throw if |
||
} | ||
|
||
var flowId = testFlow[0].z || "f1"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Running 58-debug_spec.js under the test helper gives:
Running any tests that include debug nodes give:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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}); | ||
} | ||
}; | ||
|
||
|
@@ -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(); | ||
}); | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,9 @@ | |
}, | ||
{ | ||
"name": "Mike Blackstock" | ||
}, | ||
{ | ||
"name": "Dean Cording" | ||
} | ||
], | ||
"keywords": [ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,10 @@ describe('function node', function() { | |
helper.unload(); | ||
}); | ||
|
||
after(function(done) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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":[[]]}]'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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'? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
|
There was a problem hiding this comment.
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 dependencyfoobar
.But
foobar
has a bug affecting my NR Node, and I want to fix it. I clonefoobar
from GitHub, make my changes, runnpm link
in the working copy. Then, I return to mynode-red-contrib-foobar
working copy and runnpm link foobar
. My bug fix will then be reflected innode-red-contrib-foobar
. If I make more changes tofoobar
,node-red-contrib-foobar
automatically gets them, because of the link.