nightwatchjs / nightwatch

Integrated end-to-end testing framework written in Node.js and using W3C Webdriver API. Developed at @browserstack
https://nightwatchjs.org
MIT License
11.79k stars 1.31k forks source link

Using promises with getText is not behaving as expected #529

Closed mekdev closed 9 years ago

mekdev commented 9 years ago

Hi Folks,

I am trying to implement a get data structure call on a table and building on top of getText(). I loop through iterations of rows to get multiple fields which is then returned to the call back and up into the test level.

But on resolving all the promise(s) it never gets fulfilled and waits for fulfillment before proceeding in the loop. I am using Q library to try to achieve sync-like flow inside the loop. This is in the method getAllJobPromises() which does a Q.Promise.all() on all the promises which is a getText()

/**
 * Gets the job and returns via a callback to the test
 */
this.getJobs = function(callback) {
    waitForJobListPresent();
    browser.elements('css selector', JOB_LIST_ROW, function (result) {
       var jobList = [];
        console.log(result.value.length + ' entries found');
        // Main loop going through the values, base 1 for CSS index
        for (var cssIndex = 1; cssIndex <= result.value.length; cssIndex++) {
            var jobInfo = {name: '', creator: '', date: ''};
            jobList = getAllJobPromises([getJobName(cssIndex),getJobCreator(cssIndex),getJobDate(cssIndex)], jobList);
        }
        console.log("Before main call back " + jobList);
        callback(jobList);
    });
    return browser;
};

This method just abstracts the logic of defining a anon function inside a loop above. It just takes in a list of promiss and resolves all of it before returning the result.

/**
 * Get the values from all promiss(s)
 * @param promises
 * @param arr
 * @returns {*}
 */
function getAllJobPromises (promises, jobDataArray) {
    Q.Promise.all(
        [promises[0],
        promises[1],
        promises[2]]
    ).then(function (content, err) {
        if (err) {
            console.log('err');
        } else {
            console.log(content + " --- fulfiled");
            jobDataArray.push({name: content[0], creator: content[1], date: content[2]});
            console.log('Joblist pushed');
        }
    });
    return jobDataArray;
}

All get Promise calls are implemented like this where we construct a new promise resolve the data from callback getText() and returning the promise. There are 2 more variations of this extracting 2 more fields.

/**
 * Get Job Name
 * @param index
 * @returns {*|exports|module.exports}
 */
function getJobName(index) {
    var name = new Q.Promise (function (resolve, reject) {
        browser.getText(JOB_LIST_ROW + 'td:nth-child(1)', function (result) {
            // Need to get the data from this scope
            var name = result.value;
            console.log(name + " name");
            resolve.bind(name);
        });
    });
    return name;
}

I was expecting resolving promise.all to block all promises and constructing the array. But the fulfill block never gets executed.

Console output

10 jobs found <----------- from getJobs() Before main call back <---- end of the loop before doing callback test1 name <------ in the individual promisses getJobName() test2 name test3 name

The fulfill block never gets executed "--- fulfiled" and "Joblist pushed"

I tried this pattern with the async call fs.readfiles() and it works I am not sure why it does not behave the same as our getText() call.

Was expecting this output

10 jobs found test1 name --- fulfiled Joblist pushed test2 name --- fulfiled Joblist pushed test3 name --- fulfiled Joblist pushed Before main call back

Any help is appreciated. My initial version was advised by Stephanie to to take a look at custom commands and .perform which is what I am looking into next.

cc. @sknopf

sknopf commented 9 years ago

A couple edits from original response -

The problem is your creation of jobList is not blocking. You can have getAllJobPromises return a promise, and do something like (untested):

...
browser.elements('css selector', JOB_LIST_ROW, function (result) {
  var jobList = [];
  var rowPromises = [];
  for (var cssIndex = 1; cssIndex <= result.value.length; cssIndex++) {
    var jobInfo = {name: '', creator: '', date: ''};
    rowPromises.push(getAllJobPromises(...));
  }
  Q.all(rowPromises).then(callback(jobList));
 });
...

Also you need to pass browser.elements as a callback to waitForJobListPresent

Looking at all this though it might be simpler just to use an execute instead..

this.getJobs = function(callback) {
  waitForJobListPresent(function () {
    browser.execute(function() {
      var t = document.querySelectorAll(...);
      var jobList = [];
        // for row in t
        // do some stuff
        // populate jobList as you go
        return jobList;
      }, [], callback);
    });
}
mekdev commented 9 years ago

Edited to fix typo

@sknopf thank you very much! + also thank you for the updated resolution. I was working on this last night and will continue today. I agree with the browser.execute route, it maybe the easiest since the flow is sync in browser.execute. But this code snippet is executed on the actual browser itself correct ?

sknopf commented 9 years ago

Sorry not sure I follow your question?

mekdev commented 9 years ago

Sorry, I should have provided more context. According to this - http://nightwatchjs.org/api#execute. Inject a snippet of JavaScript into the page for execution in the context of the currently selected frame.

The code injected actually runs on the browser itself correct ?

sknopf commented 9 years ago

Correct

mekdev commented 9 years ago

I got it @sknopf!!! I went with the original Q.all We didn't need the promise blocking at the getText, but actually in the loop logic.

function buildJobData (rowResults, callback) {
    // Main loop going through the values, base 1 for CSS index
    var jobList = [];
    var rowPromises = [];
    console.log(rowResults.value.length + ' entries found');
    // Main loop going through the values, base 1 for CSS index
    for (var cssIndex = 1; cssIndex <= rowResults.value.length; cssIndex++) {
        rowPromises.push(getJobRow(cssIndex));
    }
    Q.all(rowPromises).then(function(result){
        console.log("Before main call back " + result.length);
        callback(result);
    });
}

function getJobRow (index) {
    var deferred = new Q.defer();
    var jobInfo = {name: '', creator: '', date: ''};
    browser.getText(LOCATOR + ' td:nth-child(1)', function (result) {
        jobInfo.name = result.value;
        console.log(jobInfo.name + " name");
        browser.getText(LOCATOR + ' td:nth-child(2)', function (result) {
            jobInfo.creator = result.value;
            console.log(jobInfo.creator + " creator");
            browser.getText(LOCATOR + ' td:nth-child(3)', function (result) {
                jobInfo.date = result.value;
                console.log(jobInfo.date + " date");
                deferred.resolve(jobInfo);
            });
        });
    });
    return deferred.promise;
}

Then in my tests I can access the results via the callback result parameter as an array of my data :)

Thanks a lot for helping out on this, I will definitely put some of my time helping and contributing with you guys going forward!

sknopf commented 9 years ago

Nice, looks great. Thanks for posting your resolution, and yeah contribute anytime :)

PaquitoSoft commented 8 years ago

@mekdev Could you post some sample code from your tests when you execute your buildJobData function? I'm trying to understand how can I run some async assertions. In my case, after checking there are N elements in my page, I need to iterate over them running some check against each element, but I don't get it how to tell nightwatch to wait for my assertions before proceeding with the rest of the test case.

Thanks.