-
-
Notifications
You must be signed in to change notification settings - Fork 27.3k
#229 Page Object Pattern #434
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
Conversation
Review comments:
|
==== | ||
|
||
username - admin | ||
password - password No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this file used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intention was for informational purpose only.
i'll remove this file
@colinbut you have my review remarks. Please comment once you've implemented the changes and I'll take another look. |
on terms of whether it is necessary you could argue it both ways. The tests are not unit tests so my initial implementation was to put it in a separate module at least. Since, normally in real-world these are full end to end integration tests which normally resides in a separate project/module etc. But on reflection maybe it is more sensible to put into one module since those are the only tests for this example. I can certainly combine them together as you suggested. @iluwatar would you like me to do that? |
Yes, please. I think it is better that way. |
@iluwatar done changes from your initial feedback |
I have problems running the example. In Eclipse when I run the App.java I get In App.java should it be |
The license headers are missing from two files: [INFO] Updating license header in: /home/ilkka/java-design-patterns/page-object/src/main/resources/sample-ui/css/style.css |
When running tests I get this nasty looking exception although the passes. Do you know what might cause this?
|
Those are my comments for now. @colinbut looking forward to your response. |
I use Intellij IDEA for development. Intellij and Eclipse handles multi-module project very differently from each other. I've reworked that area. |
have now added the license headers to these 2 files |
Ah, this is my oversight. The test actually passes despite the exception and the build passes. This is actually caused by JaCoCo code coverage trying to instrument the underlying library class in the HtmlUnit library i'm using. This is causing a problem because that class in culprit has a massive over 4000+ lines of code in a single method. I've added an exclusion to the list of exclusions already there since i don't believe this should be part of the code coverage |
@iluwatar my responses are in comments above. |
Thanks for the latest updates. Everything seems to work as expected. Good job @colinbut 👍 |
@all-contributors please add @colinbut for code |
I've put up a pull request to add @colinbut! 🎉 |
I created a sample application sub-module which just contains the
App.java
driver class that runs the sample web app (which is just a simple web site really for demonstration purposes).App.java
does nothing special apart from just tries to open up the web app in default web browser.Another sub-module test-automation is as the name suggest a test module which contains the page objects and the test classes that uses these page objects.Normally, an automation framework/tool like Selenium will be used for test automation utilising the Page Objects but for the purpose of demonstrating this pattern implementation i've used a simple library HtmlUnit instead as Selenium is very heavyweight and i wanted the pattern example to focus on how the pattern is normally implemented.