alecxe / eslint-plugin-protractor Goto Github PK

Invalid usage:

element.all(by.repeater('product in products')).then(function(products){
    expect( products.length >= 1 ).toBeTruthy();
});

Valid usage:

expect(element.all(by.repeater('product in products')).count()).toBeGreaterThan(1);

Related: http://stackoverflow.com/questions/39043554/pulling-ng-repeat-array-in-protractor

This is to warn if absolute URLs are used in browser.get(). Recommend setting baseUrl instead.

Invalid usage:

browser.get("https://google.com/search")

Valid usage:

browser.get("/search")

This would issue a warning:

element.all(by.css(".myclass")).get(0);
element.all(by.css(".myclass")).get(-1);

This would not:

element.all(by.css(".myclass")).first();
element.all(by.css(".myclass")).last();

Compound classes are not permitted inside the by.className locator.

Example of a violation:

element(by.className('md-flex md-body-l flex'));

For examples, if I have the following locator and css in the page object:

country: this.$homeAddressPendingData.$('[data-e2e="country]')

The missing end quote breaks and prints out error message. I don't think that it is intentional but it is a good feature to have except it doesn't have line number.

It will be great to have a line number when we see breakage like that.

Problematic case:

element.all(by.repeater(arg));

Failure message:

TypeError: Cannot read property 'indexOf' of undefined

Using eslint-plugin-protractor 1.24.0 and the latest ESLint.

Invalid usage:

element.all(by.css('.items li')).filter(function(elem, index) {
  elem.getText().then(function(text) {
    return text === 'Third';
  });
}).first().click();

Valid usage:

element.all(by.css('.items li')).filter(function(elem, index) {
  return elem.getText().then(function(text) {
    return text === 'Third';
  });
}).first().click();

This needs more real-life samples. Search through SO?..

This is just to see if the plugin would not fail with an error during a static code analysis run (partially inspired by EmmanuelDemey/eslint-plugin-angular#260).

TODO: find real-world protractor projects with a good test codebase.

This is to enforce the Navigate to the page under test before each test style guide recommendation.

Sample use cases: div[class="test"] vs div.test. Or div[class*="test"] vs div[class*=test]. Should explore more example cases.

This would probably require renaming the rule.

I am using cucumber with protractor. When I apply use-first-last rule, I am getting the following error msg:

Warning: Cannot read property 'value' of undefined Use --force to continue.

I am not sure what is happening

According to the Protractor's style guide using the by.xpath() is considered a bad practice. Let's add a warning if by.xpath locator is used.

It is quite rare there is the need to write a custom function for browser.wait(). Much more often there is an existing Expected Condition for the problem. Sometimes, users just don't know that there is already a condition covering the use case.

Example of a violation:

browser.wait(function () {
    return browser.getCurrentUrl().then(function (url) {
        return url.indexOf("word") >= 0;
    });
}, 5000);

Correct usage:

browser.wait(protractor.ExpectedConditions.urlContains("word"), 5000);

Should probably turn the rule off by default.

Inspired by this SO problem: http://stackoverflow.com/questions/37925891/clear-function-not-working-in-my-automated-protractor-tests-javascript.

Example:

if (elm.isDisplayed()) {
    // ...
}

The problem is - isDisplayed() returns promise which itself is always truthy regardless of the real displayedness state. The promise has to be resolved to obtain the real true/false value.

We, of course, cannot cover all the possible problem variations, but can work with simple cases like this. Methods we should support:

• isDisplayed()
• isEnabled()
• isSelected()
• ...

no-expect-in-po rule should support minimatch options via an additional configuration key, smthlike pathsOptions similar to what eslint-plugin-disable is providing.

Selenium webdriver has deprecated getInnerHtml() and getOuterHtml() which is the reason why Protractor does not have these methods documented on the API page.

Is there that we can support cucumber step_definitions as well for this rule?

Initially proposed by Florent B. here in comments and I liked the idea as well.

executeScript() and executeAsyncScript() should never appear in specs and page objects and should rather be parts of helper and util modules. This rule would probably require a setting that defines glob patterns for specs and pos.

Inspired by https://github.com/dferber90/eslint-plugin-meteor plugin.

Bootstrap classes are usually layout-oriented and don't contain any "data"-specific information about an element.

Example of a violation:

element.all(by.css("div.col-sm-offset-6 a"));

Though, I'm not completely sure this is a good fit for this specific plugin.

We need to go through every rule and utility function and, if there is a jsdoc missing, add it. Similar to what ESLint has internally, example here.

This is to enforce this Style Guide recommendation and tackle cases when things like model, binding, repeater are used inside XPath expressions or CSS selectors, e.g.:

$('[ng-bind="some.binding"]');
$('[ng-repeat="x in y"]'); 

As opposed to a recommended approach:

element(by.binding('some.binding'));
element(by.repeater('x in y')); 

@davidadas, thanks for the suggestion!

Sometimes, it is easy to forget the "all" part and call one of the ElementArrayFinder methods on an ElementFinder, e.g.:

element(by.css(".class1")).filter(function (elm) {
    // ...
});

Here is a sample use-case: http://stackoverflow.com/questions/40733958/protractor-cant-find-elements-of-element.

If possible, all implemented rules should provide a fix function to automatically fix the determined violations. Example here.

If we have this repeater element:

<li ng-repeat="car in cars | orderBy:year"></li>

This would be considered warning:

element.all(by.repeater("car in cars | orderBy:year"));

Instead, this would be recommended:

element.all(by.repeater("car in cars"));

Example of a violation:

var MyPage = function () {
   this.grids = element.all(by.css(".mygrid"));
   this.firstGrid = element.all(by.css(".mygrid")).first();
}

Valid version:

var MyPage = function () {
   this.grids = element.all(by.css(".mygrid"));
   this.firstGrid = this.grids.first();
}

This is to discourage using ng-click, ng-change etc attributes inside CSS selectors.

TBD.

See https://travis-ci.org/lucassus/angular-webpack-seed/builds/175647512
and https://github.com/lucassus/angular-webpack-seed/blob/eslint-plugin-protractor-bug/e2e/.eslintrc.json

/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/eslint-plugin-protractor/lib/rules/valid-locator-type.js:60
  return node.arguments && node.arguments[0].type === 'Literal'
                                            ^

TypeError: Cannot read property 'type' of undefined
    at isArgumentLiteral (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/eslint-plugin-protractor/lib/rules/valid-locator-type.js:60:45)
    at EventEmitter.CallExpression (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/eslint-plugin-protractor/lib/rules/valid-locator-type.js:110:37)
    at emitOne (events.js:101:20)
    at EventEmitter.emit (events.js:188:7)
    at NodeEventGenerator.enterNode (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/eslint/lib/util/node-event-generator.js:40:22)
    at CodePathAnalyzer.enterNode (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:608:23)
    at CommentEventGenerator.enterNode (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/eslint/lib/util/comment-event-generator.js:97:23)
    at Controller.enter (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/eslint/lib/eslint.js:925:36)
    at Controller.__execute (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/estraverse/estraverse.js:397:31)
    at Controller.traverse (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/estraverse/estraverse.js:501:28)
    at Controller.Traverser.controller.traverse (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/eslint/lib/util/traverser.js:36:33)
    at EventEmitter.module.exports.api.verify (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/eslint/lib/eslint.js:922:23)
    at processText (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/eslint/lib/cli-engine.js:264:31)
    at CLIEngine.executeOnText (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/eslint/lib/cli-engine.js:754:26)
    at verify (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/gulp-eslint/index.js:20:25)
    at Transform.util.transform [as _transform] (/Users/lucassus/Projects/OpenSource/angular-webpack-seed/node_modules/gulp-eslint/index.js:70:17)

This is the follow-up to #25 but specific to CSS selectors (decided to make a separate rule):

Example - the locator of a parent element is repeated for multiple page object fields:

var MyPage = function () {
   this.parent = $(".container #parent");
   this.child1 = $(".container #parent div:first-of-type");
   this.child2 = $(".container #parent #subcontainer > .add-client");
}

The .container #parent part in this case is repeated and should be reused:

var MyPage = function () {
   this.parent = $(".container #parent");
   this.child1 = this.parent.$("div:first-of-type");
   this.child2 = this.parent.$("#subcontainer > .add-client");
}

NOTE: no violation should be reported if a direct parent-child relationship detected after a repetitive part. E.g. .container #parent > div:first-of-type instead of a .container #parent div:first-of-type in the example above

There is a common anti-pattern in Protractor when something is explicitly resolved and then asserted instead of passing a promise directly to expect() which is patched to understand promises and resolve them implicitly before making an expectation.

This should issue a warning:

element(by.css("#myid")).getText().then(function (text) {
    expect(text).toEqual("My Text");
});

This should be used instead:

expect(element(by.css("#myid")).getText()).toEqual("My Text");

I can imagine that this would not of course catch all the violations of the type, but I think, at the very least, the rule should look that the promise resolution functions argument (text in this case) is not then passed to expect()..

Recently, I've caught this problem - missed the by.css() inside the element.all():

element.all("some css selector");

This led to "Invalid Locator" error message at runtime, but I think we can warn about it earlier statically.

We should take no-redeclare rule as a base for no-shadowing rule as it approaches the problem in a simpler and more reliable way.

Inspired by this use case where first() was incorrectly called on an ElementFinder:

http://stackoverflow.com/questions/39710881/protractor-locator-issues

Sample use case: http://stackoverflow.com/questions/40881822/cannnot-focus-element-in-dropdown-test

Example of a violation:

element(by.tagName('option[value="Test"]'))

Here, a CSS selector is used as a tag name locator value.

This is to enforce the Avoid using expect() in page objects rule.

This would probably require a configurable glob pattern to match page objects js files.

Create an integration test similar to the one implemented in eslint-plugin-mocha that checks that all the defined rules are configured properly and documentation is present for every rule.

This is for rules no-angular-classes and no-bootstrap-classes.

This is to discourage nesting multiple promise resolution functions, e.g.:

elm1.getText().then(function (text1) {
  elm2.getText().then(function (text2) {
    elm3.getText().then(function (text3) {
      elm4.getText().then(function (text4) {
        // do smth with text1, text2, text3, text4
      });
    });
  });
});

Recommend using protractor.promise.all() instead:

protractor.promise.all([
  elm1.getText(),
  elm2.getText(),
  elm3.getText(),
  elm4.getText()
]).then(function (texts) {
   // do smth with texts
});

The rule should probably have the nestedness level configured.

From time to time, I see the incorrect element, element.all chaining like in this topic.

Invalid usage:

var dates = $('#provider_details_view').element.all(by.repeater('repeater name');

Valid usage:

var dates = $('#provider_details_view').all(by.repeater('repeater name');