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

yield statement instead of nested callback #367

Closed
bitcity opened this issue Dec 3, 2014 · 14 comments
Closed

yield statement instead of nested callback #367

bitcity opened this issue Dec 3, 2014 · 14 comments

Comments

@bitcity
Copy link

bitcity commented Dec 3, 2014

Writing tests is quite painless with Webdriverio, thanks to the well documented API. Although after a while, I feel my code could be more succinct & readable if there was something like Yiewd for WebdriverIO, It gets rid of nested callbacks with yield statements. The library is quite small and on the surface it looks like it might be straightforward to port it. Any thoughts?

Here's an example of its usage with wd : https://github.com/admc/wd#yiewd

P.S. I'm sure the developers are well aware of this but I couldn't find any reference in the issues section, hence bringing it up.

@christian-bromann
Copy link
Member

Hi @addremove
I understand, sometimes it is a little bit verbose but I am almost done with bringing promises into WebdriverIO. I think it will highly improve the way we write test:

var chai = require("chai");
var chaiAsPromised = require("chai-as-promised");
chai.use(chaiAsPromised);
chai.should();
chaiAsPromised.transferPromiseness = client.transferPromiseness;

client
    .remote(options)
    .init()
    .url('http://www.google.com')
    .getTitle().should.eventually.equal("Google"); // true
    .end();

Also no nested callbacks:

client
    .commandA('input').then(function(res) {
        if(res.value.length > 2) {
            return client.commandB();
        } else {
            return client.commandC();
        }
    }).then(function(res) {
    // ...
    });

What do you think?

FYI if you are interested check out the WebdriverIO promise branch. It is almost stable. Only deep assertions with Chai doesn't work yet (e.g. client.getTitle().should.become('something'))

@christian-bromann
Copy link
Member

so tests will look like

describe('test', function() {

    before(function(done) {
        client.init(done).url('http://webdriverjs.christian-bromann.com/');
    });

    it('test', function() {
        return client.getTitle().should.eventually.equal('WebdriverJS Testpage');
    });

    after(function(done) {
        client.end(done);
    });

});

@bitcity
Copy link
Author

bitcity commented Dec 3, 2014

Thank you for the sneak peek. I'll definitely check out the branch.
Specifically, I'm facing a lot of nested loops when I want to pick 'nth' element based on a class and then check some conditions (something like using client.elements from your example here #153 (comment))

Consider this HTML fragment, where I want to click a particular span, only if it's title matches a desired value.

<div class="table">
    <div class="row">
        <div class="col">
            <span class="label" title="A">Label A</span>
        </div>
    </div>
    <div class="row">
        <div class="col">
            <span class="label" title="B">Label B</span>
        </div>
    </div>
    <div class="row">
        <div class="col">
            <span class="label" title="C">Label C</span>
        </div>
    </div>
</div>

If I want to get element n, and test it's title against val, currently, I would write it like this:

client.elements('.label', function(err, result){
    client.elementIdAttribute(result[n].ELEMENT, 'title', function(title){
        if (title==val){
            client.elementIdClick(result[n].ELEMENT);
        }
    })
})

I was wondering if this can be simplified to something like.

if ( client.getAttribute('.label:eq("+n+")', 'title') == val ) {
    client.click('.label:eq("+n+")')
}

I read in the issues that you are also working on integrating Sizzle. That should also simplify some of the code above (i.e. directly fetching nth element using ".class:eq(n)" ).

@thibaut-sticky
Copy link

@addremove for your particular case, you can try: `client.click('.label[title="value"]'). This css selector will select the element with class label and with title attrique equal to value.

@christian-bromann
Copy link
Member

@addremove @thibaut-sticky or using xpath like
client.click('//span[@title="C"] and contains(@class ,"label")')

Looking forward to have a simplification for this coming in WebdriverIO. xPath is really powerful (even though it's slow).

@christian-bromann
Copy link
Member

Well but the idea to support yielding sounds great, so I'll keep that open. PR's are welcome!

@bitcity
Copy link
Author

bitcity commented Dec 3, 2014

@thibaut-sticky : I specifically want nth element, and then I want to check if it's title matches a string. What you are suggesting would be useful when I want any element whose title matches the string.

@christian-bromann Yes, XPath would help. I'll try out to see if it also supports indexing. (I was subconsciously avoiding XPath due to performance reasons, and this slipped off my mind)

@christian-bromann christian-bromann added this to the v3 milestone May 18, 2015
@christian-bromann
Copy link
Member

Since each command returns with a unique promise in v3 now, it won't be much effort to make yielding work with WebdriverIO. We should add something in our docs though to make people clear about how to use it properly.

@christian-bromann
Copy link
Member

Got following example working in the v3 branch:

var run = require('monocle-js').run;
var webdriverio = require('./index');
var client = webdriverio.remote({
    desiredCapabilities: {
        browserName: 'phantomjs'
    }
});

run(function*() {
    yield client.init().url('http://localhost:8080/test/site/www/');
    var title = yield client.getTitle();
    console.log(title);
    yield client.end();
});

The runner basically just need to overwrite the it as well as the hook methods.

@christian-bromann
Copy link
Member

Ok so this will be available in v3. All child processes will run in harmony mode automatically. People have three different methods to handle asynchronicity:

the good old callback way:

describe('my feature', function() {
    it('will do something', function(done) {
        browser
             .url('http://google.com')
             .getTitle().then(function(title) {
                 console.log(title);
              })
              .call(done);
    });
});

using promises:

describe('my feature', function() {
    it('will do something', function() {
        return browser
             .url('http://google.com')
             .getTitle().then(function(title) {
                 console.log(title);
              });
    });
});

the new fancy generator way:

describe('my feature', function() {
    it('will do something', function* () {
        yield browser.url('http://google.com');
        var title = yield browser.getTitle()
        console.log(title);
    });
});

@bitcity
Copy link
Author

bitcity commented May 26, 2015

👍 for generators ! Would love to try it out for my next project. Thank you for the good work.

@steveharman
Copy link

+1 for generators Christian :-)

@talarari
Copy link

are generators supported in v4? or is the fiber implementation used instead?

@christian-bromann
Copy link
Member

We use fibers in v4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants