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

added mouseover, resize and scroll #244

Merged
merged 21 commits into from
Jun 23, 2023
Merged

Conversation

aryangupta701
Copy link
Contributor

@aryangupta701 aryangupta701 commented Mar 8, 2023

Fix #243

Added mouseover, resize and scroll methods to handle hovering, resizing window and scrolling.

@aryangupta701
Copy link
Contributor Author

@psiinon Please guide me how can I test the behaviour of the newly added methods? I am not sure weather this will work or not.

@psiinon
Copy link
Member

psiinon commented Mar 8, 2023

Sure :)
The way I would do it is to:

  1. create a suitable test page
  2. write a simple Zest script in ZAP which interacts with it in a basic way
  3. run it in ZAP (to make sure it works)
  4. save the script
  5. run it from the command line (this is to make sure everything is still ok)
  6. add one of the new statements which will do something detectable
  7. run the script again from the cmdline - you should see the effect
  8. repeat for the other statements

@aryangupta701
Copy link
Contributor Author

okay thank you.

@thc202
Copy link
Member

thc202 commented Mar 8, 2023

Also worth looking at the codebase (e.g. ZestClientScreenshotUnitTest) to add unit tests.

@aryangupta701
Copy link
Contributor Author

how can I run the script from the command line ?

@psiinon
Copy link
Member

psiinon commented Mar 8, 2023

Theres https://github.com/zaproxy/zest/wiki/Commandline but I havnt tried it in a while - give it a go and let us know if you have any problems...

@aryangupta701
Copy link
Contributor Author

aryangupta701 commented Mar 8, 2023

I have tested it manually and it is working as expected. I am not able to understand what kind of unit test will it need to be written. Please guide me through it. I have seen ZestClientScreenshotUnitTest but still not sure about what kind of tests are needed.

@thc202
Copy link
Member

thc202 commented Mar 8, 2023

The tests you did manually do them automatically :) If that's not possible verify that at least the expected classes are called, other behaviour than the invoke itself should be tested as well.

Edit: To be clear I do think that all 3 statements being added can be tested automatically.

@aryangupta701
Copy link
Contributor Author

Thankyou @thc202

@aryangupta701
Copy link
Contributor Author

@thc202 I have written test for scrolling as of now. Please check if I went into the right direction, then I will write for others as well.

@aryangupta701 aryangupta701 changed the title added mouseout,mouseover and scroll added mouseover and scroll Mar 16, 2023
@aryangupta701 aryangupta701 force-pushed the add-new-commands branch 5 times, most recently from dc7058d to 6aad95a Compare March 16, 2023 16:10
@aryangupta701 aryangupta701 changed the title added mouseover and scroll added mouseover, resize and scroll Mar 18, 2023
@aryangupta701 aryangupta701 force-pushed the add-new-commands branch 2 times, most recently from cc40007 to 2688696 Compare March 18, 2023 18:31
@psiinon
Copy link
Member

psiinon commented Mar 20, 2023

@aryangupta701 - looking good so far, let us know when its ready to review again.

@aryangupta701
Copy link
Contributor Author

okay sure.

@aryangupta701
Copy link
Contributor Author

I do think that these methods along with already present methods will be sufficient to handle all the user interactions. If you think I am missing something please tell me.

@thc202
Copy link
Member

thc202 commented Mar 30, 2023

#244 (comment)

IMO a scroll to statement would be easier to use in most cases (though the scroll element might still be useful/needed in some cases).

#244 (comment)

This one was not yet addressed, the invoke methods are not being tested.

@thc202
Copy link
Member

thc202 commented Apr 3, 2023

You don't need to mock the Actions, you have both the WebDriver and the WebElement to verify the interactions. If that's too much work you can run the actual statement (e.g. ZestClientLaunchUnitTest.testHtmlUnitLaunch()) and verify the outcome.

@aryangupta701
Copy link
Contributor Author

okay thanks. Will I need to create a test website to test mouseover statement?

@thc202
Copy link
Member

thc202 commented Apr 4, 2023

Yes, you can serve static data like done in the test mentioned earlier (e.g. check before method and ServerBasedTest).

@aryangupta701
Copy link
Contributor Author

okay.

@aryangupta701 aryangupta701 force-pushed the add-new-commands branch 2 times, most recently from 7d749ad to 7b618fb Compare April 5, 2023 05:48
@aryangupta701
Copy link
Contributor Author

aryangupta701 commented Apr 24, 2023

@thc202 Could you please review and tell me if anything needs to be changed?

Copy link
Member

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

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

LGTM just a question about license/header for the team

Comment on lines +1 to +3
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
Copy link
Member

Choose a reason for hiding this comment

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

@psiinon @thc202 should we be using a different header now?

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine either way.

Copy link
Member

Choose a reason for hiding this comment

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

I guess thats a separate discussion - I dont mind too much either way either 😉

Signed-off-by: aryangupta701 <garyan447@gmail.com>
Signed-off-by: aryangupta701 <garyan447@gmail.com>
Signed-off-by: aryangupta701 <garyan447@gmail.com>
Signed-off-by: aryangupta701 <garyan447@gmail.com>
Signed-off-by: aryangupta701 <garyan447@gmail.com>
Signed-off-by: aryangupta701 <garyan447@gmail.com>
Signed-off-by: aryangupta701 <garyan447@gmail.com>
Signed-off-by: aryangupta701 <garyan447@gmail.com>
Signed-off-by: aryangupta701 <garyan447@gmail.com>
Signed-off-by: aryangupta701 <garyan447@gmail.com>
Signed-off-by: aryangupta701 <garyan447@gmail.com>
@thc202 thc202 merged commit 7667e34 into zaproxy:main Jun 23, 2023
@thc202
Copy link
Member

thc202 commented Jun 23, 2023

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add support for commands such as scrolling and hovering over client element
4 participants