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

Add missing examples in headless engine protocol actions #1108

Merged
merged 13 commits into from
Oct 14, 2021
Merged

Add missing examples in headless engine protocol actions #1108

merged 13 commits into from
Oct 14, 2021

Conversation

pmareke
Copy link
Contributor

@pmareke pmareke commented Oct 10, 2021

Fixes #1102

Done:

  • ActionScreenshot
  • ActionTimeInput
  • ActionSelectInput
  • ActionFilesInput
  • ActionWaitLoad
  • ActionGetResource
  • ActionExtract
  • ActionSetMethod
  • ActionAddHeader
  • ActionDeleteHeader
  • ActionSetBody
  • ActionKeyboard
  • ActionSleep
  • ActionWaitEvent (I'm not able to figure out why the test crashes)
  • ActionDebug (I don't how to test it because this action sets the internal trace to true but it's private)

Signed-off-by: pedro.lopez.mareque@gmail.com

@ehsandeep ehsandeep linked an issue Oct 10, 2021 that may be closed by this pull request
Copy link
Contributor

@forgedhallpass forgedhallpass left a comment

Choose a reason for hiding this comment

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

Please

  • solve the linter issue
  • Optional: conflict that was introduced by my refactor.
  • Optional: you could try to follow the same approach to remove duplicated code, otherwise I will do it myself later on.

v2/pkg/protocols/headless/engine/page_actions_test.go Outdated Show resolved Hide resolved
@pmareke
Copy link
Contributor Author

pmareke commented Oct 13, 2021

Please

  • solve the linter issue
  • Optional: conflict that was introduced by my refactor.
  • Optional: you could try to follow the same approach to remove duplicated code, otherwise I will do it myself later on.

Sure, I'll give it a try!

@pmareke
Copy link
Contributor Author

pmareke commented Oct 13, 2021

  • Optional: you could try to follow the same approach to remove duplicated code, otherwise I will do it myself later on.

Regarding this one I'm not quite sure what do you mean. I can see yo created a privaste helper method to use in both t.Run(), is this what you want?

@forgedhallpass
Copy link
Contributor

@pmareke I've extracted the common logic into a separate method so that I can re-use it instead of duplicating the code. The advantage is that this way the the test cases are easier to read and if later on, they'd require adjusting, they would only have to be modified in one place. But as I said, since it's test code, this is rather a challange then a requirement :-) The PR will be approved without it as well.

@pmareke
Copy link
Contributor Author

pmareke commented Oct 13, 2021

@pmareke I've extracted the common logic into a separate method so that I can re-use it instead of duplicating the code. The advantage is that this way the the test cases are easier to read and if later on, they'd require adjusting, they would only have to be modified in one place. But as I said, since it's test code, this is rather a challange then a requirement :-) The PR will be approved without it as well.

Ou no no it's perfect, the only place where we have another t.Run() it's TestActionScript, I can aded there for sure.

@pmareke
Copy link
Contributor Author

pmareke commented Oct 13, 2021

@forgedhallpass Refactored TestActionScript test

@forgedhallpass forgedhallpass merged commit 84fb341 into projectdiscovery:dev Oct 14, 2021
@forgedhallpass
Copy link
Contributor

@pmareke good job with the tests and thank you for your contribution!

To see what I've meant with regards to the 3rd action point, please see: 4d14063

@pmareke
Copy link
Contributor Author

pmareke commented Oct 14, 2021

@pmareke good job with the tests and thank you for your contribution!

To see what I've meant with regards to the 3rd action point, please see: 4d14063

Thanks so much and sorry for not understand what you said.

Take care!

@forgedhallpass forgedhallpass added hacktoberfest-accepted Status: Completed Nothing further to be done with this issue. Awaiting to be closed. labels Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Status: Completed Nothing further to be done with this issue. Awaiting to be closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] Add missing headless actions tests
2 participants