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

Add fallback for `clearValue` command #4028

Closed garg3133 closed 5 months ago

garg3133 commented 7 months ago

Description of the bug/issue

It's been reported multiple times in the past that the browser.clearValue() command does not work as expected in a few cases (see #4026).

And although this is actually an issue with the webdrivers, we should still consider adding a fallback to the clearValue command until the issue is resolved at the webdrivers end. Doing so would help Nightwatch users not waste time trying to figure out what went wrong with their test and how to fix it, while the issue exists in the webdriver.

Steps to reproduce

  1. npm init nightwatch@latest <project-name> (with defaults)
  2. cd <project-name>
  3. Add a new test file named clearValueTest.js inside test folder, with the sample test provided below.
  4. Run npx nightwatch ./test/clearValueTest.js
  5. The test will run and you'll notice that the 'hello' value didn't get cleared and 'hi' got appended to it.

Sample test

describe('clearValue Demo', function() {
  before(browser => {
    browser
      .navigateTo('https://sandbox.mabl.com/mailbox');
  });

  it('shows issue with clearValue command', async function(browser) {
    browser
      .sendKeys('input[name=to]', 'hello')
      .clearValue('input[name=to]')
      .sendKeys('input[name=to]', 'hi')
      .pause(3000);
  });

  after(browser => browser.end());
});

Potential Fix

Inside the clearElementValue method of lib/transport/selenium-webdriver/method-mappings.js file, after running the clear() command, check if the element still has some value, and if it does, send BACK_SPACE keys to the element till it is cleared.

The element variable in the above method represents the WebElement class of Selenium.

Nightwatch.js Version

3.4.1

Node Version

No response

Browser

Chrome, Edge and Safari. Works fine on Firefox.

Operating System

No response

Additional Information

No response

Kartikeya-09 commented 7 months ago

I want to work on this issue could you assign this issue to me.

adityakhattri21 commented 7 months ago

Hi @garg3133 I would like to work on this issue. Can you please assign it to me ?

garg3133 commented 7 months ago

Hi, please go through GSoC Contributor Guidance for Nightwatch. It contains details on how the issues will be assigned.

FireNdIce3 commented 7 months ago

Hi @garg3133 I propose finding the value using element.getAttribute(value) and run a loop sending back_space keys till it becomes zero Thanks

JuliusMieliauskas commented 7 months ago

Hello, I tried the method @FireNdIce3 suggested and it works just fine. Using backspaces is not the most efficient way especially if the text field is lengthy, but it is definitely a viable approach. I could fix it, if I was assigned :)

niladrix719 commented 7 months ago

Hi, I do think the same, sending back_space keys is not the best approach, rather we can select all text within the input field, i.e:-

const inputSelector = 'input[name=to]';

await browser.execute(function (inputSelector) {
   document.querySelector(inputSelector).select();
}, [inputSelector]);

and then press the delete key ;)

await browser.keys(['Delete']);

garg3133 commented 7 months ago

Hey, thanks for all the inputs.

Considering how webdrivers work and that the real issue here due to which the clearValue command is not working in the first place has to do with the command not emitting correct events that it should, we'd prefer using sendKeys here instead of going the JS way, which is known to emit the events correctly across all browsers.

Moreover, using sendKeys() to send BACK_SPACE keys would work on mobile apps as well, while the JS solution won't.

Now, I agree that using a for loop is not the best solution, so I'd suggest to directly send an array of Key.BACK_SPACEs with array size equal to the length of input field value. Key can be imported from the selenium-webdriver package.

@FireNdIce3 Let us know if you'd like to take this up since you were the first one to suggest a potential solution.

FireNdIce3 commented 7 months ago

@garg3133 Thanks a lot I'll take this issue

FireNdIce3 commented 7 months ago

Hi @garg3133 I have written the code, but I am facing one issue After doing the changes, the web driver isn't getting updated (the tests are being carried out without getting updated) Can you please help me with this Apologies for asking a simple doubt

garg3133 commented 7 months ago

Hi, no worries! I don't fully understand what you mean by "the web driver isn't getting updated". What issue are you facing exactly?

FireNdIce3 commented 7 months ago

Apologies Actually when I am doing the changes in the selenium methods they aren't actually getting changed when I'm running the tests I tried clearing cookies but the issue still persists tweaking other methods is giving the same result

garg3133 commented 7 months ago

@FireNdIce3 No need to apologise. Which selenium methods are you talking about? If you're talking about the selenium-webdriver package, then you should not change anything in there, the only change you need to make is in the Nightwatch project.

Also, how have you set up the project? Did you fork and clone this project or did you use npm init nightwatch@latest command? If you've set up this project, you need to run the example tests present in the project itself to test your changes as mentioned here: https://github.com/nightwatchjs/nightwatch/blob/main/CONTRIBUTING.md#submitting-a-pull-request

FireNdIce3 commented 7 months ago

Hi! I have forked and cloned it and used npm init nightwatch@latest only. However after doing the changes in methods-mapping.js file There is no change been reflected after running the tests. I think I am missing something here because I tried changing several methods for testing (like addValue) but its still the same Can you please help with this?

FireNdIce3 commented 7 months ago

for reference I did the following changes

async clearElementValue(webElementOrId) {

          const element = this.getWebElement(webElementOrId);
          await element.clear();
          var value = await element.getAttribute(value);
          const backArr = Array(value.length()).fill(Key.BACK_SPACE);
          await element.sendKeys(...backArr);
          return null;
        }

I even tried to throw an error to check whether the test will throw error or not but it's still running fine Sorry to ask again, but Can you please tell me where I am going wrong? Thanks

garg3133 commented 7 months ago

Hey, I understand your issue now. If you've already setup the Nightwatch project, you didn't need to run npm init nightwatch@latest command inside the Nightwatch project, as that command is for starting a new boilerplate project with nightwatch installed, and you don't want to install nightwatch package inside the Nightwatch project :)

An easy fix should be to revert all the changes in package.json and package-lock.json file, delete node_modules and run npm i again.

Then use node ./bin/nightwatch <test-file-path> --env chrome command to run the example test. The changes you make should reflect in that case.

FireNdIce3 commented 7 months ago

Hi @garg3133 Thank you so much for being so patient and for dealing with my silly mistake 😬 I ran the tests. It is running fine I am making a PR Thanks again for being patient

garg3133 commented 7 months ago

@FireNdIce3 Cool

FireNdIce3 commented 7 months ago

Please refer issue #4028 fix : added fallback for clearValue command #4035 PR Thanks a lot for guiding @garg3133 Hoping to contribute more through GSOC!

botPuneet commented 7 months ago

Hello @garg3133, I added a pr for this issue waiting to review. i also added unit test. And also resolve the issue with the last time PR. thanks for guidance . PR image

Ashu463 commented 6 months ago

Hey @garg3133 I have raised a relevant PR doing changes that you mentioned. Please review it and let me know if any changes required. thanks

garg3133 commented 5 months ago

Fixed in #4035.