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

issue #4028 fix : added fallback for clearValue command #4035

Merged
merged 20 commits into from Apr 18, 2024

Conversation

FireNdIce3
Copy link
Contributor

@FireNdIce3 FireNdIce3 commented Feb 22, 2024

Add a fallback for clearValue command where Nightwatch will now try to delete the text in an input field by sending backspaces if the text is not deleted by the clear command directly.

This fallback will also work for commands like setValue, updateValue, etc. which clears the text before entering new text.

@CLAassistant
Copy link

CLAassistant commented Feb 22, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

github-actions bot commented Feb 22, 2024

Status

  • ✅ Type files updated!

Copy link
Member

@garg3133 garg3133 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd also need to have some unit tests written for this PR. You can try to explore how the tests are written in Nightwatch and ask if you don't understand anything or need any help.

lib/transport/selenium-webdriver/method-mappings.js Outdated Show resolved Hide resolved
lib/transport/selenium-webdriver/method-mappings.js Outdated Show resolved Hide resolved
@FireNdIce3
Copy link
Contributor Author

We'd also need to have some unit tests written for this PR. You can try to explore how the tests are written in Nightwatch and ask if you don't understand anything or need any help.

Can you please guide me where to explore regarding writing tests?
Thanks

@FireNdIce3
Copy link
Contributor Author

I am confused why they are not strictly equal
Can you please help with this?

@garg3133
Copy link
Member

I am confused why they are not strictly equal
Can you please help with this?

Two arrays are never strictly equal (being strictly equal would mean that both refer to the same memory location). Use can use deepStrictEqual instead.

@FireNdIce3
Copy link
Contributor Author

FireNdIce3 commented Feb 25, 2024

I looked through the verbose logs and indeed keys are going in an array
But I am still unsure what's wrong with the test
is there any Doc i can refer for this?

@FireNdIce3
Copy link
Contributor Author

hi the tests are working fine as of now.. is there anything that's left?
thanks

@FireNdIce3
Copy link
Contributor Author

FireNdIce3 commented Feb 27, 2024

@garg3133 the linting issues has been fixed
apologies fixing linting issues.. thankyou for being patient

@garg3133
Copy link
Member

@FireNdIce3 Can you remove the formatting changes you've made in the test file? Your PR should not contain any changes that are not related to the issue at hand, it makes it really difficult to review the PR.

@FireNdIce3 FireNdIce3 requested a review from garg3133 March 1, 2024 08:41
Copy link
Member

@garg3133 garg3133 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is still not right (it isn't actually testing anything). A good way to see if tests are doing anything or not is to revert back your code changes and see if the test is failing. If the test is still passing then that means that the test is actually not testing what you meant to test, i.e., not doing anything.

In the test you've written, when you run it you can see in the logs that the last response is 404.

The motive of writing this test is to make sure that the .getProperty('value') command is being run and after that .sendKeys() is also being run. But to test if these commands are actually executed, we'd somehow need to check if the mocks defined by us are being used.

We can do that by adding a onRequest() method in mock. So, what we'll do is something along the lines of following:

      const bArr = Array(6).fill(Key.BACK_SPACE);
      const str = bArr.join('');
      
      let sendKeysMockCalled = false;

      MockServer.addMock(
        {
          url: '/session/13521-10219-202/element/5cc459b8-36a8-3042-8b4a-258883ea642b/clear',
          response: {value: 'sample'}
        },
        true
      );

      MockServer.addMock({
          url: '/session/13521-10219-202/element/5cc459b8-36a8-3042-8b4a-258883ea642b/value',
          method: 'POST',
          postdata: {
            str,
            value: bArr
          },
          onRequest() {
            sendKeysMockCalled = true;
          },
          response: {
            value: null
          },
          statusCode: 200
        }, true);

      client.api.clearValue('#webdriver', function (result) {
        assert.strictEqual(result.value, null);
        assert.strictEqual(sendKeysMockCalled, true);
        // ensures fallback is working
      });

      client.start(done);

The above test will make sure that the .sendKeys() command is actually called, which is the purpose of having this test.

@FireNdIce3
Copy link
Contributor Author

@garg3133 sorry for that..
will do the required changes

@garg3133
Copy link
Member

garg3133 commented Mar 1, 2024

No need to be sorry. Are you looking into some other issues as well?

@FireNdIce3
Copy link
Contributor Author

@garg3133 yes, I am looking into the issues you mentioned to work upon. currently exploring the codebase and investigating the cause.

@FireNdIce3
Copy link
Contributor Author

hi @garg3133
I have updated the test and its passing
Can you please review again when you get the time
Thanks

Comment on lines 178 to 179
client.api.sendKeys('css selector', '#webdriver', bArr, function callback(result) {
assert.strictEqual(sendKeysMockCalled, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you calling .sendKeys() command directly? We just want to call .clearValue command from outside and then we want to test if the internal element.sendKeys() (which you specified in method-mappings.js file) was called or not if the element.getProperty('value') command still returns some value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @garg3133 ,
for some reason, the post request was not being sent when I tried checking the logs
So I looked into the codebase to see how keys were being sent in a test and I saw this method
I tried doing it by using .sendKeys() and it worked

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @garg3133 , for some reason, the post request was not being sent when I tried checking the logs So I looked into the codebase to see how keys were being sent in a test and I saw this method I tried doing it by using .sendKeys() and it worked

Are you still working on this issue ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I am working on this. had been sick that's why the delay
apologies

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, just wanted to make sure that this is all still being worked on. I could land an eye if you need. (not sure that I can be of a great help but I can still try 😬)

Copy link

Status

  • ❌ No modified files found in the types directory.
    Please make sure to include types for any changes you have made. Thank you!.

@garg3133 garg3133 merged commit fcaabc1 into nightwatchjs:main Apr 18, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants