Write Lint Checks for the End-to-End Tests #11496

U8NWXD · 2020-12-25T00:48:37Z

To improve the quality of our end-to-end testing code, we want to create lint checks for common bad practices. If you would like to help with this issue, please leave a comment on this issue with the task you want to work on.

How to Write E2E Lint Checks

First think about how you can identify the bad practice based solely on the text of the code. Imagine you get a code file as a string. How would you look for any cases of the practice you are writing a check against? While you won't know the types of variables in the code, you can take advantage of linters' abstract syntax trees representing the parsed code. Note that you might conclude it's not possible to enforce a practice with a linter. For example, we discovered that with action.clear. If this happens, please let us know so we can remove it from the list.
Follow the instructions on the linter wiki page for adding a new lint check. Since you're linting the E2E code, you'll want to write a custom ESLint check. You'll probably do something like this:
- Write the code for your check as a new file in scripts/linters/custom_eslint_checks/rules/.
- Write a corresponding *.spec.js file to test your code.
- Add your rule to to the object with oppia/e2e-action in overrides of .eslintrc
Remove the node_modules/eslint-plugin-oppia/ directory and run yarn install to install your linter.
Run python -m scripts.run_custom_eslint_tests to check that the tests you wrote pass.

Run your lint checks with:

python -m scripts.linters.pre_commit_linter --path core/tests/protractor
python -m scripts.linters.pre_commit_linter --path core/tests/protractor_desktop
python -m scripts.linters.pre_commit_linter --path core/tests/protractor_utils

and fix any errors.

Create a PR to add your lint check!

You can also check out #11734 as an example of how to add these checks. If you have any questions, please reach out to @U8NWXD.

Lint Checks to Add

Notes

It isn't possible to write a lint check for using action.clear instead of calling .clear() on an element. This is because it's valid to call .clear() on rich text editors.

The text was updated successfully, but these errors were encountered:

* Add lint check for e2e action scripts * Add exclude list for end-to-end-action-checks lint test * Fix end-to-end-action-checks to exclude by base filename * Shorten action checks rule name * Fix e2e-action lint failure * Limit e2e-action checks to e2e files * Move e2e-action file include/exclude lists to .eslintrc * Add files to e2e-action exclude list * Remove Lint Check for action.clear Rich text editors have a .clear() function, so we cannot use a lint check to find cases where we want to use action.clear instead. * Remove checks for clear from e2e-action.spec.js * Use function name as messageId in e2e-action lint check * Remove mistakenly committed tmp.py file * Remove unnecessary newline from .eslintrc * Simplify e2e-action lint logic * Replace elem.sendKeys with action.select2 * Remove click before select2

RituCs · 2021-03-04T07:46:39Z

Hi , I am a new here and would like to contribute on this issue.Please guide me further.

U8NWXD · 2021-03-05T03:37:23Z

Welcome @RituCs! Could you pick one of the tasks at the top of this issue? Then I'll assign it to you

RituCs · 2021-03-05T05:55:43Z

Thank You @U8NWXD.I have no idea which issue is good for me. I know Python and little bit Javascript.

RituCs · 2021-03-06T19:24:08Z

Welcome @RituCs! Could you pick one of the tasks at the top of this issue? Then I'll assign it to you

@U8NWXD Please guide me what I have to do.I want to start contribute.

U8NWXD · 2021-03-08T03:27:19Z

@RituCs these lint checks are going to be entirely in Javascript. For more python-based lint checks, take a look at #8423.

…1734) * Add lint check for e2e action scripts * Add exclude list for end-to-end-action-checks lint test * Fix end-to-end-action-checks to exclude by base filename * Shorten action checks rule name * Fix e2e-action lint failure * Limit e2e-action checks to e2e files * Move e2e-action file include/exclude lists to .eslintrc * Add files to e2e-action exclude list * Remove Lint Check for action.clear Rich text editors have a .clear() function, so we cannot use a lint check to find cases where we want to use action.clear instead. * Remove checks for clear from e2e-action.spec.js * Use function name as messageId in e2e-action lint check * Remove mistakenly committed tmp.py file * Remove unnecessary newline from .eslintrc * Simplify e2e-action lint logic * Replace elem.sendKeys with action.select2 * Remove click before select2

AdityaDubey0 · 2021-03-17T19:58:49Z

I am working on Do not use browser.sleep() calls part of the issue.

U8NWXD · 2021-03-19T02:02:50Z

@AdityaDubey0 I assigned you! Let me know if you have any questions

…#12248)

Mayank-gaur · 2021-03-31T21:50:50Z

Hi @U8NWXD , I am working on the subtask 'Making sure constant variable names are in all-caps ' of issue #8423. I created a js file and wrote my rule code in it, and a spec.js file in which I wrote my tests. My test code is like this: 'const a = 5'. When I ran 'python -m scripts.run_custom_eslint_tests' to check my linter against my test cases, I got an error 'A fatal parsing error occurred: Parsing error: The keyword 'const' is reserved'. Can you please guide me what am I doing wrong?
Also, Can you please confirm exactly where do we have to add our rule in .eslintrc file?

Is it below line 98 or line 117?

U8NWXD · 2021-04-01T00:45:17Z

@Mayank-gaur I think this should be added below line 117 since I think you'll be able to fix all the lint failures your rule introduces. The e2e-action rule is listed separately because it can't be applied everywhere until #10798 is finished.

Regarding your parsing error, I haven't seen that before. @Hudda might be able to help more--they have more experience with the linter. You might try testing out your rule with https://astexplorer.net/. I found it very helpful for writing lint rules

Mayank-gaur · 2021-04-01T09:36:42Z

Thanks @U8NWXD. I have one more query please guide me. I added my rules in 'opensource/oppia/scripts/linters/custom_eslint_checks/rules' directory, still when I am adding it to eslintrc file, should I add it as 'oppia/rule-name : "error"' or should I use the full path 'opensource/oppia/scripts/linters/custom_eslint_checks/rules/rule-name' ?

U8NWXD · 2021-04-02T04:04:07Z

You should use oppia/rule-name, not the full path

Mayank-gaur · 2021-04-02T08:26:32Z

Thanks @U8NWXD !

sajalasati · 2021-04-03T10:13:36Z

@U8NWXD Checked the lint check Do not use browser.sleep() calls as its PR #12248 has been been merged around 13 days ago.

SAEb-ai · 2021-04-07T15:35:33Z

@U8NWXD Can you please assign me Do not call .first() on a list of ElementArrayFinders

iabouara24 · 2021-04-07T22:10:50Z

Hi, I'm a first-time contributor and I'd like to attempt some of these issues. Could you please assign me "Do not call .last() on a list of ElementArrayFinders" and "Do not access .length of ElementArrayFinders"?

suryasiriki4 · 2021-05-01T10:16:25Z

@U8NWXD, can you assign me do not use filter?

Varun8216889 · 2021-05-02T17:44:58Z

Hey,
Myself Varun Dagar and I am fresher. I am looking for good first issue to start contributing.
So if u assign some issue to me. It would be great.

MA86 · 2021-05-03T00:30:48Z

@U8NWXD, My lint-check, which disallows use of .then() method, works as expected. It introduces 1000+ failures where .then() is used. Most of these failures are in Ignored Files.

My question: I added my rule in line 119 of .eslintrc file. Should I remove it from there and add it next to oppia/e2e-action instead? Or perhaps create a new spot for it? Thanks a bunch!

UPDATE: Repositioning my rule with oppia/e2e-action fixed failures in Ignored Files.

U8NWXD · 2021-05-03T21:21:42Z

@MA86 yes, please add it next to oppia/e2e-action. .then() is fine in the rest of the code base, we just don't want it in the e2e tests

U8NWXD · 2021-05-03T21:22:07Z

@suryasiriki4 done

U8NWXD · 2021-05-03T21:22:56Z

@Varun8216889 I've assigned you to Do not access .length of ElementArrayFinders.

Varun8216889 · 2021-05-04T06:20:57Z

thank u

suryasiriki4 · 2021-05-05T14:07:19Z

@U8NWXD, can you assign me do not use filter?

@U8NWXD, I have 2 questions :

What is the valid replacement for .filter(), can you tell me where can I find it?
Is it to be checked only in protractor_utils dir?

thank you.

U8NWXD · 2021-05-06T02:55:51Z

@suryasiriki4 Regarding (1) I don't think there's a drop-in replacement. We'd expect to use something like a for loop instead. Regarding (2) this should also be checked in protractor/ and protractor_desktop (though you probably won't find many violations there). Following the same structure as #11734 should handle this correctly

MA86 · 2021-05-10T05:21:46Z

@U8NWXD, Do not use .then() is now successfully merged. Thanks for the help! Now I would like to tackle Do not index into ElementArrayFinders with bracket notation ([]), can you assign me to it?

U8NWXD · 2021-05-11T02:54:03Z

@MA86 done!

SAEb-ai · 2021-05-26T04:39:00Z

@U8NWXD May I please be assigned to Do not use browser.switchTo().activeElement(). Thanks!

U8NWXD · 2021-05-28T00:27:26Z

@SAEb-ai done (also, I think you can assign yourself now since you're a collaborator!)

MA86 · 2021-06-10T01:01:36Z

Hi @U8NWXD,

Do you happen to know:

Does ElementArrayFinder behave like an array? Like, myArray[2] = 4?
Can I assume that all references to ElementArrayFinder object is stored in a variable called elementArrayFinder?

FYI, while searching the codebase, I found 45 places in 8 different files where elementArrayFinder was used.

Thanks a bunch!

U8NWXD · 2021-06-21T17:13:47Z

@MA86 sorry for the late reply. Yes, an ElementArrayFinder does behave like an array. For a variable myArray of type ElementArrayFinder, myArray[2] is valid syntax. However, we also use async-await, which is not compatible with treating an ElementArrayFinder like an array. Therefore for Oppia code, we should not treat an ElementArrayFinder like an array. See the E2E wiki page for more info.

No, we should not assume that all ElementArrayFinder objects are called elementArrayFinder. In fact, they often have more descriptive names like explorationCards.

MA86 · 2021-06-23T01:42:03Z

@U8NWXD, thanks for the answers! I'll take a fresh look at this issue and see how far I go.

Aakash-Raj-2001 · 2021-08-29T07:47:02Z

@U8NWXD Hi, I would like to work on this issue. This is my first open-source contribution so please assign me a file accordingly.

U8NWXD · 2021-08-30T01:47:03Z

@Aakash-Raj-2001 it looks like you've asked to be assigned in a bunch of issues. For your first contribution, please pick one to start with

vashuteotia123 · 2021-09-01T05:42:33Z

@U8NWXD Hi, I would like to work on this issue. And being totally new to this environment I really don't know how to being. All issue seems to be assigned, Can I get any issue assigned to me? Also a bit of resource on how to being would be of great help.

U8NWXD · 2021-09-03T00:35:51Z

Deassigned @suryasiriki4 @ahreehong @iabouara24 @SAEb-ai @Varun8216889 @MA86 due to inactivity. If you are still working on your tasks, let me know and I'll re-assign you

ruinan-liu · 2021-09-03T00:41:44Z

@U8NWXD Hi, this is my first open source project and I would like some guidance on getting started. Do you know of any issue I can begin working on? Thanks

U8NWXD · 2021-09-03T00:44:23Z

@ruinan-liu I assigned you to "Do not use filter". @vashuteotia123 I assigned you to "Do not use forEach."

For guidance on how to get started with the project, see our wiki page. To get started on this issue in particular, see the instructions at the top

ruinan-liu · 2021-09-08T11:32:22Z

@U8NWXD Hi there, I have a quick question regarding my check. So I updated in the protractor-practices.js a rule to check that no array uses the .filter() function, it passes the test python -m scripts.run_custom_eslint_tests when using the example I gave in protractor-practices.spec.js. However when I run the following python -m scripts.linters.pre_commit_linter --path core/tests/protractor_utils it shows that in the protractor/util there are code that are using the .filter() function and test failed. Should I change the code that are currently using the .filter() function? I don't think there are any built in alternatives for .filter()?

U8NWXD · 2021-09-08T20:56:06Z

@ruinan-liu I responded on gitter 😄 Here's what I said:

To answer your questions, you can use a for loop as a replacement for .filter(), and yes, you need to fix the files that are currently failing the lint check as part of your PR. See step 5 here: https://github.com/oppia/oppia/wiki/Lint-Checks#how-to-add-new-lint-checks

U8NWXD added this to Triage in Automated QA Team via automation Dec 25, 2020

U8NWXD moved this from Triage to E2E Tests Backlog in Automated QA Team Jan 1, 2021

U8NWXD mentioned this issue Jan 19, 2021

Fix Part of #11496: Lint Check for E2E Action Functions #11734

Merged

4 tasks

U8NWXD added the good second issue label Feb 23, 2021

U8NWXD self-assigned this Feb 23, 2021

U8NWXD moved this from E2E Tests Backlog to In Progress in Automated QA Team Feb 23, 2021

U8NWXD changed the title ~~E2E Lint Checks~~ Write Lint Checks for the End-to-End Tests Feb 23, 2021

U8NWXD added good first issue and removed good second issue labels Feb 27, 2021

AdityaDubey0 mentioned this issue Mar 17, 2021

Fix part of #11496: Adds lint check to disallow browser.sleep() calls #12248

Merged

4 tasks

sajalasati mentioned this issue Mar 18, 2021

Implement new linter checks #8423

Open

71 tasks

AdityaDubey0 mentioned this issue Apr 8, 2021

Fix part of #11496 : Adds lint checks disallow activeElements #12467

Closed

4 tasks

MA86 mentioned this issue Apr 30, 2021

Fix Part of #11496: Add lint check for .then() method #12677

Merged

4 tasks

vashuteotia123 mentioned this issue Sep 10, 2021

Fix part of #11496: Lint Check for no use of forEach #13843

Closed

4 tasks

vashuteotia123 mentioned this issue Sep 10, 2021

Fix part of #11496: Lint Check for no use of forEach #13844

Open

4 tasks

ruinan-liu mentioned this issue Sep 10, 2021

Fix Part of #11496: Lint Check for .filter() functions #13848

Closed

4 tasks

ruinan-liu mentioned this issue Sep 10, 2021

Fix Part of #11496: Lint Check for filter Functions #13849

Open

4 tasks

oppia / oppia Public

Sponsor oppia/oppia

Write Lint Checks for the End-to-End Tests #11496

Write Lint Checks for the End-to-End Tests #11496

Comments

U8NWXD commented Dec 25, 2020 • edited

How to Write E2E Lint Checks

Lint Checks to Add

Notes

RituCs commented Mar 4, 2021

U8NWXD commented Mar 5, 2021

RituCs commented Mar 5, 2021

RituCs commented Mar 6, 2021

U8NWXD commented Mar 8, 2021

AdityaDubey0 commented Mar 17, 2021

U8NWXD commented Mar 19, 2021

Mayank-gaur commented Mar 31, 2021

U8NWXD commented Apr 1, 2021

Mayank-gaur commented Apr 1, 2021

U8NWXD commented Apr 2, 2021

Mayank-gaur commented Apr 2, 2021

sajalasati commented Apr 3, 2021

SAEb-ai commented Apr 7, 2021

iabouara24 commented Apr 7, 2021

suryasiriki4 commented May 1, 2021

Varun8216889 commented May 2, 2021

MA86 commented May 3, 2021 • edited

U8NWXD commented May 3, 2021

U8NWXD commented May 3, 2021

U8NWXD commented May 3, 2021

Varun8216889 commented May 4, 2021

suryasiriki4 commented May 5, 2021

U8NWXD commented May 6, 2021

MA86 commented May 10, 2021

U8NWXD commented May 11, 2021

SAEb-ai commented May 26, 2021 • edited

U8NWXD commented May 28, 2021

MA86 commented Jun 10, 2021

U8NWXD commented Jun 21, 2021

MA86 commented Jun 23, 2021

Aakash-Raj-2001 commented Aug 29, 2021

U8NWXD commented Aug 30, 2021

vashuteotia123 commented Sep 1, 2021

U8NWXD commented Sep 3, 2021

ruinan-liu commented Sep 3, 2021

U8NWXD commented Sep 3, 2021

ruinan-liu commented Sep 8, 2021

U8NWXD commented Sep 8, 2021

U8NWXD commented Dec 25, 2020 •

edited

MA86 commented May 3, 2021 •

edited

SAEb-ai commented May 26, 2021 •

edited