GalenPages.extendPage overwrites elements instead extend it

[JaniszM]

Hi.

Looks like when I create some page using $page or GalenPages.extendPage and I use this page object to again extend page using GalenPages.extendPage then the primary and secondary fields are overwirtten not extended :/ I was expecting that all page will be extended also with webElements but GalenPages.extendPage just overwrittes it.

E.g:

var BasePage = $page("Some base.", {
    logout: "xpath: //a[@ng-click='logout()']",
    footerMenu: "xpath: //footer//div[@class='g-container']/div[3]",
    content: "xpath: //div[@class='b-wrapper']//header//section"
});

this.ExtendedPage = function (driver) {
    var basePage = new BasePage(driver);

    // HERE TEST LOGS
    var root = basePage._primaryFields;
    console.log("root type " + typeof root);
    console.log("root type " + root.constructor.name);
    for (var i = 0; i < root.length; i++) {
        console.log("elem type " + typeof root[i]);
        console.log("elem type " + root[i].constructor.name);
        console.log(root[i]);
    }

     GalenPages.extendPage(basePage, driver, "some extended page", {
        search: "xpath: //input[@type='search']",
        searchButton: "xpath: //button[@class='search-form-submit btn']",
    });

    // HERE TEST LOGS
    var root = basePage._primaryFields;
    console.log("root type " + typeof root);
    console.log("root type " + root.constructor.name);
    for (var i = 0; i < root.length; i++) {
        console.log("elem type " + typeof root[i]);
        console.log("elem type " + root[i].constructor.name);
        console.log(root[i]);
    }
    return basePage;
};

Check this out guys, this looks like serious bug :(

[ishubin]

Ah, so you mean it overwrites the _primaryFields and _secondaryFields ? Yeah, that's a stupid issue :) thanks for reporting it! I will fix it on master branch once I have more time

[ishubin]

I think is line is incorrect https://github.com/galenframework/galen/blob/master/galen-core/src/main/resources/js/GalenPages.js#L103 It should merge those two fields instead of replacing them

[JaniszM]

Yeah, looks like this is the problem :D

[JaniszM]

@ishubin I think you should update documentation also. This is because in the doc you have proposed a form this.ExtendedPage = function (driver) { to create a page. This results in creating object in the global context (usually). This mean that after fixing this bug, each test build this way will fail now if there are more than one "extends" with different primary elements on the page ;) Anotherword, this fix breaks backward compatibility I think. But this is critical fix.

When do you plan to release v2.4? :)

[ishubin]

Not sure why that fix should break anything? All I plan to fix is merging the _primaryFields and _secondaryFields on the extended page only, and not on the base page. So the idea is if you plan to extend some base page - your extended page should get both fields correctly. And by the way, those two fields are just array of strings. That fix should only effect the waitForIt function. Also the way $page and extendPage function work is by generating a constructor function, and I think it is totally safe to bring it to the global context. At the same time instantiated page should be kept in the local scope. I agree about the general this.??? = pattern that it is awful and should be replaced with explicit export. But that is a separate issue.

As for release 2.4 - I am not sure yet, as I am also working on other projects. But this particular issue I plan to fix in 2.3.

[JaniszM]

Yes, breaking compatibility is about waitForIt. I am not 100% sure but I think if someone will use a global context this.??? to call constructor then the one will merge all his constructor calls to the global context and the waitForIt obviously will end in timeout for waiting elements that do not belong to the page (all pages merged to global context). That I meant :)

[JaniszM]

I have found one more issue when calling extendPage: function (page, driver, name, mainFields, secondaryFields) {. Any custom property created on page that should be extended is not copied to the new Page object but should be I suppose. What if I want an additional nice function ? :)

After these thoughts I think extendPage copies properties in bad direction. You are creating new page first and copying properties from old to new. This is not extending in fact :D You should create new instance of the page and from these instance update (copy?) all necessary properties on original page. This would be an extending page :)

[ishubin]

@JaniszM I think you have a point. To be fair, GalenPages was originally created when I didn't have enough experience with JavaScript. Right now I would probably rewrite it completely because it became such a mess and it is hard to maintain it. Perhaps I could even use babel to recompile it from ES6 into ES5 so that it is easier to develop it.

[hypery2k]

@ishubin I could support you with this. Maybe it would help to use TypeScript during development and then generate ES5-JavaScript

[JaniszM]

If I find time I can also support you with this. Just let me know :)

[ishubin]

@hypery2k @JaniszM Thanks for proposal! TypeScript could be a good idea. Though I wouldn't jump into it so fast. The reason is I plan to extend Galen Framework into a standalone service so that it could be used from other languages and test frameworks. If I manage to do this properly, then we wouldn't even need GalenPages and complex test suite executors in Galen. For instance we could just move GalenPages into a separate lib so that it could be used in node. I wanted to do this a long time ago, just haven't yet thoroughly thought through all the details. My end goal is to simplify Galen so that it becomes more flexible and easier in terms of maintenance.

[hypery2k]

So like somekind of local Network service which Receiver rfc like method calls?

[ishubin]

Well I plan to make it a simple web server with json-based api. I have already made something like this but for a different purpose: https://github.com/ishubin/galen-ide In the end we would just need to build clients on other platforms and those will talk to galen service and vice-versa.

[JaniszM]

Good idea but remember that still you need some language to describe test senarious. Currently I am using js. With proper test project design it is possible to build very complex tests :)