Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Generic text field #61

Closed
wants to merge 7 commits into from

Conversation

nicopaul
Copy link
Contributor

@nicopaul nicopaul commented Dec 7, 2016

Added all functionalities of the PageObject GenericTextField:
-PageObject GenericTextField
-GenericTextFieldTest
-IntegrationTest
-Assert
-AssertTest
-Matcher
-MatcherTest


This change is Reviewable

@nicopaul nicopaul force-pushed the GenericTextField branch 3 times, most recently from 402ffb9 to a90532a Compare December 9, 2016 11:37
@bitterblue
Copy link
Member

Looks good to me :)
Can you please update the documentation (support-hamcrest.md and support-assertj.md)?

@@ -54,6 +55,10 @@ protected String getHTMLFilePath() {

// UTILITY

protected void assertPageObjectCanBeInitialized(PageObject object) {
Copy link
Member

Choose a reason for hiding this comment

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

I cannot find any usage of this assertion within the PR. Why did you add it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need this assertion for other PageObjects which are based on GenericTextField.
That is why I added it on this branch, so I dont have to add it on all the other Branches.

Copy link
Member

Choose a reason for hiding this comment

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

I looked at some of the branches in your repo (EmailField, UrlField, SearchField) and didn't find any references to the method. Can you point me to a branch, where you are actually using it? I don't want to integrate dead code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The function assertPageObjectCanBeInitalized is on my master branch and not on my GenerixTextField Branch, maybe thats why it is irritating.
I need this function in The Following Classes/Branches:
GenericList --> GenericListIntegrationTest
OrderedList --> OrderedListIntegrationTest
UnorderedList --> UnorderedListIntegrationTest
Form --> FormIntegrationTest
Paragraph --> ParagraphIntegrationTest

I thought about adding this test for the validation of mapping/class to the GenericTextField-Classes. If I should add it, I can do this on Monday.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!
I'll keep this commit out, but accept the rest of your pull request. You can add the commit to a future PR where the test method is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay

*
* @since 1.2
*/
public class GenericTextFieldMatcher extends PageObjectMatcher {
Copy link
Member

Choose a reason for hiding this comment

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

And one more thing: please turn the constructor private (instead of protected), so that codacy is happy?

@bitterblue
Copy link
Member

Nico, I merged your backport of GenericTextField. Thanks, and don't forget to pull the current master branch!

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

Successfully merging this pull request may close these issues.

2 participants