noamtcohen / SlimJS

An Async Node.js SliM server for FitNesse
17 stars 9 forks source link

Need help with chaining promises using SlimJS #6

Closed ilyagutman closed 8 years ago

ilyagutman commented 8 years ago

Hello Noam,

Thank you for another, very useful extension to Fitnesse, much appreciated. I am planing on actively using your framework. However, my tests will look too tightly coupled and too verbose if I don't figure out how to better take the advantage of Promises in the context your framework. In one of your examples you write "If you want to do something asynchronous, return a thenable (promise)". This is exactly where I began. However, I also need to be able to chain these "then" functions to be able to keep my tests concise. For example, let's say my verification method captures the output from a System Under Test. Now, I need to return it to Fitnesse for comparison, just like in your example, but also chain it with the other downstream method(s) that will perform the additional validations on the different fields and/or perform a clean-up job, like shutting down a server and/or deleting prerequisite records in the database. How would you implement such asynchronous chaining using your framework? Could you please post another example? Your help is much appreciated.

Thank you Ilya Gutman

noamtcohen commented 8 years ago

I'm not sure I understand but this is what I think. Lets say you have a test and you need to get a value from a promise returned from the SUT:

function MyTest(){
   this.getResult = function(){
      return{
         then:function(fulfil,reject){
            var sut = new MySut();
            var asyncTask = sut.getPromise();
            asyncTask.then(fulfil,reject);
         }
      }
   }
}

This can be abbreviated to:

function MyTest(){
   this.getResult = function(){
       return (new MySut()).getPromise();
   }
}

Does this answer your question?

ilyagutman commented 8 years ago

Thank you for your help Noam. Based on your example I put together this sample test that does fit me syntactically well.

Ilya Gutman

function FunctionsChainingExample() {

        this.execute = function (cmd) {
                return {
                        then:function(fulfill, reject) {
                                method1(fulfill).then(fulfill, reject).then(function() {
                                        method2();
                                }).then(function() {
                                        method3();
                                });
                        }
                }
        }
}

function method1(resolve) {
        return new Promise(function(resolve) {
                resolve("called method one");
                console.log("called method one");
        });
}

function method2() {
        return new Promise(function() {
                console.log("called method two");
        });
}

function method3() {
        return new Promise(function() {
                console.log("called method three");
        });
}

module.exports.FunctionsChainingExample = FunctionsChainingExample;
noamtcohen commented 8 years ago

Great. But I think that there might be an issue, it could be that SlimJS will exit before your async operation ends since you are calling method2 & method3 after the promise "uses" SlimJS's callbacks. In your example it's not a problem since method2 & method3 are sync operations.

ilyagutman commented 8 years ago

Good point Noam. This was just an example as for each use case we will have a slightly different fixture. In this case the last method means to clean up after the test, hence it is the last and the synchronous. In some cases with the multiple asynchronous actions in the same test I use Promose.all that holds me from exiting, etc.

Thank you for all your help Ilya

noamtcohen commented 8 years ago

@ilyagutman I'm tying to understand your code. method1() is returning a promise. The promise is constructed with a callback that take a callback as it's argument. After that you call the then function with the same callback with some kind of hidden implementation and you continue chaining two more thens. I'm trying to rephrase:

then:function(fulfill, reject) {
    var aPromise = new Promise(function(fulfill) {
        fulfill("called method one");
        console.log("called method one");
    });

    var promise2 = aPromise.then(fulfill, reject);
    // at this point the code returned to FitNesse by fulfil being called
    // in the task that was defined when constructing the promise above.

    // It seems that promise2 should have some task defined that would eventually trigger 
    // the callback in this then function,  but where is that happening?
    var promise3 = promise2.then(function() {
        return new Promise(function() {
            console.log("called method two");
        };
    });

    // Again, who is triggering the argument of this then?
    var promise4 = promise3.then(function() {
        return new Promise(function() {
            console.log("called method three");
        };
    });
}
ilyagutman commented 8 years ago

This is probably due to my inexperience with the JavaScript in general and with the Promises in particular. All I am after is the ability to chain asynchronous (and synchronous) methods into a single chain of executions, using named methods, so in a way they serve as a documentation for this test making it readable. It would be almost impossible if I didn't overcomplicated or oversimplified in the process.

noamtcohen commented 8 years ago

Ok, but it seems to me that method2() and method3() are never called?

noamtcohen commented 8 years ago

SlimJS doesn't return a promise from it's fulfil method because it doesn't make scenes. After returning to FitNesse it's over, there is nothing more to do, the test is concluded. The callback provided by SlimJS should be the last one called. I'm no promise expert either but maybe this seems correct?

function FunctionsChainingExample() {

    this.execute = function (cmd) {
        return {
            then:function(fulfill, reject) {
                method1().then(method2).then(method3).then(fulfill);
                // FitNesse would have "called method three" as the result.
            }
        }
    }
}

function method1() {
    return new Promise(function(resolve) {
        console.log("called method one");
        resolve("called method one");
    });
}

function method2(resolutionFromMethod1) { // "called method one"
    return new Promise(function(resolve) {
        console.log("called method two");

        resolve("called method two");
    });
}

function method3(resolutionFromMethod2) { // "called method two"
    return new Promise(function(resolve) {
        console.log("called method three");

        resolve("called method three");
    });
}

module.exports.FunctionsChainingExample = FunctionsChainingExample;
ilyagutman commented 8 years ago

This is the example you asked me to post - the actual working test based on the examples I was speculating about earlier. I guess the main issue I was (and maybe still is) experiencing is the ability to mix asynchronous and synchronous methods with the strict order of their execution. In this test, I synchronously start the server (our internal server); start the system under test (SUT) that outputs something, and then shut down that server when the successful SUT output either found or not found. As I understand, I have to return a Promise from the fire() method, in order to be able to chain it later with the method that shuts down my server. This construct will more or less accommodate most of our test cases. What sticks out for you here?

Thank you

function PluginManagerProcessMessage(argv) {

    this.execute = function (command) {

        var server = startServer();

        return {
            then: function (fulfill, error) {
                var resolve, reject;
                fire(command, resolve, reject).then(function (value) {
                    fulfill(extractOutput(value));
                }).then(function (value) {
                    stopServer(server);
                });
            }
        }
    }
}

function fire(command, resolve, reject) {

    const exec = require('child_process').exec;

    return new Promise(function (resolve) {
        exec(command, {cwd: '/home/sevone/remote-poller/Test/FakeClient'}, function (error, stdout, stderr) {
            if (error) {
                return reject(error);
            }

            resolve(stdout.trim());
        });
    });

}

function extractOutput(value) {

    const ndx = value.indexOf("Success");
    if (ndx > -1) {
        var arr = value.substring(ndx).split(" ");
        const retval = arr[0].trim() + trimnl(arr[1]) + arr[2];
        console.log(retval);
        return retval;
    }

    return "Success token not found";
}

function trimnl(value) {
    return value.replace(/\r?\n|\r/g, " ");
}

function startServer() {
    var spawn = require('child_process').spawn;
    const server = spawn('node', ['server.js'], {cwd: '/home/sevone/remote-poller/RelayStreamer/'});

    console.log("server was started");

    return server;
}

function stopServer(server) {
    server.kill('SIGINT');
    const text = "server was stopped";
    return text;
}

module.exports.PluginManagerProcessMessage = PluginManagerProcessMessage;
noamtcohen commented 8 years ago

I don't think your code will work. for a bunch of reasons.

Look through the code bellow it has all the answers you need to chain sync and async functions. Like in the code bellow I would put all your functions in side promises: startServer, fire, extractOutput, stopServer

Create a file called STAM.js:

function FunctionsChainingExample() {

    this.execute = function (cmd) {
        return {
            then:function(fulfill, reject) {
                method1().then(method2,reject).then(method3,reject).then(fulfill,reject);
                // FitNesse would have "called method three" as the result.
            }
        }
    }
}

function method1() {
    return new Promise(function(resolve,reject) {
        console.log("called method one");
        resolve("called method one");
    });
}

function method2(resolutionFromMethod1) { // "called method one"
    return new Promise(function(resolve,reject) {
        console.log("called method two");

        resolve("called method two");
    });
}

function method3(resolutionFromMethod2) { // "called method two"
    return new Promise(function(resolve,reject) {
        console.log("called method three");

        resolve("called method three");
    });
}

var testFixture = new FunctionsChainingExample();

testFixture.execute().then(function(result,reject){
   console.log(result);
});

run

node STAM.js

//called method one
//called method two
//called method three
//called method three
ilyagutman commented 8 years ago

I like this. This is clean, readable right off the main method, methods themselves are separated and reusable, the flow is very clear on what follows what etc. I really like this and will re-implement my fixture this way. Thank you again.

noamtcohen commented 8 years ago

You will encounter two issues trying to keep this clean, the server object and the parsed output. Have fun ;-)

noamtcohen commented 8 years ago

Ilya wanted to show you something I did today on a different project. For this example I changed to your context. Run and look at the example and w'll talk later. Please update SlimJS to 2.1.1 Paste this to a FitNesse test page:

|import|
|ilya  |

|script     |server|
|$SERVER=   |self  |
|startServer|true  |

|script   |commander  |
|$CMD_OUT=|fire|Gutman|

|script|parser                        |
|note  |This is the fire parsed output|
|check |parse    |$CMD_OUT    |Hi!    |

|script  |$SERVER|
|shutdown|true   |

Create this test code, ilya.js:

var http = require('http');

function server(){
    this.httpServer;
}

server.prototype={
    self:function(){
        return this;
    },

    startServer:function(){
        return{
            then:function(fulfil,reject){
                this.httpServer =http.createServer();
                fulfil(true);
            }
        }
    },

    shutdown:function(){
        this.httpServer.close();
        return true;
    }
}

function commander(){}
commander.prototype ={
    fire:function(cmd){
        return {
            then:function(fulfil,reject){
                fulfil("??? Hi! "+cmd+"???");
            }
        }
    }
}

function parser(){}
parser.prototype={
    parse:function(data){
        return{
            then:function(fulfil,reject){
                fulfil(data.substr(4,3));
            }
        }
    }
}

module.exports={
    server:server,
    commander:commander,
    parser:parser
}

Reference: http://fitnesse.org/FitNesse.SuiteAcceptanceTests.SuiteSlimTests.ChainWithInstanceTest http://fitnesse.org/FitNesse.SuiteAcceptanceTests.SuiteSlimTests.ChainTest

ilyagutman commented 8 years ago

Noam, thank you. I absolutely will try it tomorrow morning. I am having a little difficulty following the syntax just by eyeballing it, but from what I gather, you manage the strict sequence of execution not by chaining methods in the fixture using .then(), but instead in the test itself. Is this correct? If I understood the idea correctly, it definitely works much better, as it guarantees that the server won't shutdown via the asynchronous call while I am still parsing a message. After I run it tomorrow, I will have a better understanding of how things actually flow.

Thank you Ilya

noamtcohen commented 8 years ago

Using the prototype is more memory efficient.

this:

function server(){
    this.httpServer;
}

server.prototype={
    self:function(){
        return this;
    },

    startServer:function(){
        return{
            then:function(fulfil,reject){
                this.httpServer =http.createServer();
                fulfil(true);
            }
        }
    },

    shutdown:function(){
        this.httpServer.close();
        return true;
    }
}

Is more or less like this:

function server(){
    this.httpServer;

    this.self = function(){
        return this;
    }

    this.startServer = function(){
        return{
            then:function(fulfil,reject){
                this.httpServer =http.createServer();
                fulfil(true);
            }
        }
    }

    this.shutdown = function () {
        this.httpServer.close();
        return true;
    }
}
noamtcohen commented 8 years ago

@ilyagutman close this issue?

ilyagutman commented 8 years ago

Yes. Thank you very much Noam for all your help with this. Ilya