-
Notifications
You must be signed in to change notification settings - Fork 7
PC-325 Integration Tests with squished commits #113
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
base: master
Are you sure you want to change the base?
Changes from all commits
9130aef
00b4b0a
9e4212c
1d8fa87
b732ae2
b7a2f64
8854feb
691ccc7
68714dd
120843a
a2be246
078207a
06522a5
af4063c
d7a167a
3a6cb4b
4f8d81a
2c232a2
fd32741
f1b82c6
810feda
d6a45a4
c39d221
52a7b75
5a01594
5350b7a
e4f2c90
ae026ab
80eb101
769ed55
a921098
8f9a42a
b2da48a
16a58dd
f52b316
51781bf
8d084a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| target/* | ||
| .idea/* | ||
| .allure/* |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| # Integration Tests for PhenomeCentral | ||
|
|
||
| ##### End-to-end tests on a native web browser using Selenium | ||
|
|
||
| Jira story link: https://phenotips.atlassian.net/browse/PC-325 | ||
|
|
||
| ## Major Maven Dependencies | ||
| - [Selenium WebDriver](https://www.seleniumhq.org/projects/webdriver/) | ||
| - [WebDriverManager](https://github.com/bonigarcia/webdrivermanager) | ||
| - [TestNG](https://testng.org/doc/index.html) | ||
| - [Allure](http://allure.qatools.ru/) | ||
|
|
||
| ## Requirements | ||
| - JDK 1.8 or above. The codebase uses some Java 8 features such as lambdas. | ||
| - phenomecentral.org (parent project) already built. Currently tested with: `1.2-SNAPSHOT using PT 1.4.4` | ||
|
|
||
| ## Usage | ||
| ### Quick start | ||
| - Clone this repository | ||
| - Build phenomecentral.org using `mvn install` as usual | ||
| - `cd standalone/target` and then `ls`. Ensure that one `phenomecentral-standalone*.zip` is present. | ||
| - `cd ../../end-to-end-tests/scripts` and then `./runIntegrationTests.sh` to run locally with default environment variables. | ||
|
|
||
| This script will extract the PC standalone zip to a subfolder in `end-to-end-tests/target/instances`, start up the PC instance, run the tests, and generate an Allure report. The script stops the instances and SMTP upon a SIGINT (Ctrl-c). | ||
| By default, it will start PC on port 8083, with stop port 8084, run the tests using Chrome, have emails listened to on port 1025 and the email inbox page on port 8085. | ||
|
|
||
| ### Custom Environment | ||
| `runIntegrationTests.sh` accepts several arguments to specify several (optional) variables. `./runIntegrationTests.sh --help` for details. | ||
| Example: `./runIntegrationTests.sh --browser chrome --start 8083 --stop 8084 --emailUI 8085` | ||
|
|
||
| If you *already* have a running instance of PC up and *running* (whether local or elsewhere): | ||
| - Don't use the runIntegrationTests.sh | ||
| - cd into the `end-to-end-tests` folder | ||
| - Ensure there is a fake SMTP server listening. If not and it is a local instance: `java -jar scripts/fake-smtp/MockMock.jar -p 1025 -h 8085` | ||
| - Run: `mvn test -Dsurefire.suiteXmlFiles=src/test/java/org/phenotips/endtoendtests/testcases/xml/AllTests.xml -Dbrowser=DESIRED_BROWSER -DhomePageURL=PC_INSTANCE_URL -DemailUIPageURL=EMAIL_UI_URL` replace `DESIRED_BROWSER` with one of `chrome, firefox, safari, edge, or ie` and `PC_Instance_URL` with the URL of the running instance and `EMAIL_UI_URL` with the URL to access the email inbox | ||
| - Example: `mvn test -Dsurefire.suiteXmlFiles=src/test/java/org/phenotips/endtoendtests/testcases/xml/AllTests.xml -Dbrowser=chrome -DhomePageURL=localhost:8083 -DemailUIPageURL=localhost:8085` | ||
|
|
||
|
|
||
| ## Opening the Allure Report | ||
| - Report is stored in `target/site/allure-maven-plugin/`. Open `index.html` with a browser *other than Chrome* to view the report. | ||
| - Chrome disables loading resources from different origins or protocols by default. See http://biercoff.com/opening-local-version-of-allure-report-with-chrome/ | ||
|
|
||
| ## Running on IE | ||
| - These instructions need to be followed before IE can be used. Notably, you must manually disable "Enhanced Protected Mode" in the IE's settings before automated test software such as Selenium is able to interact with an IE browser. See https://github.com/SeleniumHQ/selenium/wiki/InternetExplorerDriver#required-configuration | ||
|
|
||
| ## Test Assumptions | ||
| - The test suite assumes a blank PC instance. | ||
| - Certain tests check for emails. Ensure that the PC instance is setup to allow for emails to be sent. If running locally, that would mean a fakeSMTP service has started. Even if we don't run tests for emails (there are only two or three), we still need a working SMTP to approve new users | ||
|
|
||
|
|
||
| ## Architecture | ||
| - The design of the test suite follows a PageObject model/design pattern. | ||
| - Classes in the `pageobjects` package contain selectors for a page. It also contains methods that interact with a single webpage. For instance, in the `LoginPage` class, there is a method to type in a passed username, password, and then click on the login button. These call native Selenium methods to interact with the page, such as clicking and typing. | ||
| - `BasePage` is an abstract class that contains common methods for every page such as logging out. I.e. it contains what is on the toolbar that appears on every page. All other page inherit it. | ||
| - Classes in the `testcases` define endtoend test cases. A class should contain a "theme" of tests. Most tests should be able to run individually given that the `SetupUsers` class has been run once for the PC instance. See individual classes for details. | ||
| - `BaseTest` is an abstract class that handles what happens before and after each test. Using the `@Before/After(Test/Suite)` annotations, it handles the initiation of the web browser and screenshots on failure. All other test classes inherit it. | ||
| - Classes in the `common` package contain classes or interface that are used in both test cases and pageobjects. | ||
| - Notably, we have `CommonPatientMeasurements` which defines measurements assigned to a patient. A CommonPatientMeasurements object can be constructed during a test case as something to verify against, and it can also be constructed in a page object class to define what is on the View Patient form. | ||
| - The interfaces provide enums that are used in various classes to avoid passing in long strings for a selection dropdown. | ||
| - TestNG provides a `@Test` annotation to allow for running test methods and classes directly in an IDE by providing a main function. In intelliJ, you should see an interactable green run triangle on the test method declaration line. | ||
| - `BasePage` also contains environment variables that can get updated via parameters passed in the JVM. Certain variables such as the PC instance URL can be modified directly if you want to run these tests from the IDE without supplying environment variables. | ||
| - Change the variables `HOMEPAGE_URL`, `ADMIN_USERNAME`, `ADMIN_PASS`, etc. as needed | ||
|
|
||
| - XML files that define a test suite are located in `testcases/xml`. TestNG uses one of these XML files to execute a test suite during `mvn test`. The default is `NoTests.xml`. | ||
| - By default, these end-to-end tests will not run on a build of phenomecentral.org during the `mvn install` or `mvn test` lifecycle. This prevents failure of the entire build if even one end to end test fails. Instead, we have to pass a different XML file with the `-Dsurefire.suiteXmlFiles` flag and run `mvn test` explicitly. See Usage. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's not a good idea, the default should be to run all tests. If we want maven builds to be fast locally, we disable the whole module via profiles.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. Profiles should be the solution to my earlier concern then. The concern I had was that anyone building |
||
| - If you run `mvn test` or `mvn install` within the `end-to-end-tests` directory, notice how no Selenium tests are run | ||
| - The XML defines the order the tests should be run in. Instead of using `@dependsOnMethods`, `@Groups`, or `@priority` in the testcases code, we define the desired order on the XML. Each of those mentioned annotations creates a global variable that can conflict with each other and cause undefined behaviour if you are not careful. For example, don't mix `@priority` with `@dependsOnMethods` as the order of importance is not well defined in that case. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having to modify both the source code and an extra XML file will lead to errors. Test methods should not depend on a particular order of execution, each test should be standalone and:
As such, each test should be complete, starting with creating a new patient, and if it needs two or more pieces of information, such as measurements needing a birthdate to be entered, then all needed information should be entered in the same test method.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While some of them are already standalone, I grew concerned about the lengthened time it was taking to run tests when assumptions can't be made. For example, creating a patient manually and importing a patient JSON are two separate tests. For a matching test, I could just use these two patients, or I would have to create another two again which adds another minute or so for a detailed patient. This additional time adds up to something non-negligible with enough test cases. It will be no problem changing it back to completely standalone tests. I had this debate to myself at one point and do agree that standalone is ideal. |
||
| - Allure has an attached default listener with the `@step` annotation. Each step in a test can be seen as a call to a method from a pageObject class. | ||
|
|
||
| Flow of execution: | ||
| - runIntegrationTests.sh script called: | ||
| - Parses command line arguments (browser, start, stop ports, etc.) supplied to it | ||
| - Extracts phenomecentral-standalone.zip found in parent's standalone/target folder (../../standalone/target), | ||
| - Starts instance and SMTP | ||
| -> script calls `mvn test -Dsurefire.suiteXmlFiles=src/test/java/org/phenotips/endtoendtests/testcases/xml/AllTests.xml -Dbrowser=SUPPLIED_BY_SCRIPT -DhomePageURL=SUPPLIED_BY_SCRIPT -DemailUIPageURL=SUPPLIED_BY_SCRIPT` | ||
| -> Selenium tests run | ||
| -> Script stops PC instance and SMTP | ||
| -> Allure report generated using another call to mvn | ||
| - Upon SIGINT, script stops the instance and SMTP before exiting. | ||
|
|
||
| ## Suggestions for future | ||
| - Implement the ability to make REST calls to generate users using the API. This might reduce the requirement of a fakeSMTP and allows users to be created much more easily | ||
| - JMeter load testing | ||
| - Investigate video recording, custom listener class or adapter to generate test and handle failure | ||
| - Multiple measurement entries, selectors and methods only work with the first measurement entry | ||
| - Other features that are bound to change: Entering data to Patient Form, Pedigree Editor, and Match Table | ||
|
|
||
| ## Limitations | ||
| - UI/UX issues can't be detected effectively. Ex. Element is present but is in 1 pt font or hard to see. | ||
| - A seperate machine or server is highly recommended for test stability. While Selenium does not explicitly capture the mouse or keyboard, the browser window might come into focus if a native OS error dialogue (such as the one we get when being prompted for unsaved changes) pops up. Use with caution. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <module type="JAVA_MODULE" version="4" /> | ||
sdumitriu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
sdumitriu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| <project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xmlns="http://maven.apache.org/POM/4.0.0" | ||
| xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
|
|
||
| <groupId>org.phenomecentral</groupId> | ||
sdumitriu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| <artifactId>phenomeCentral-automation</artifactId> | ||
| <version>1.0-SNAPSHOT</version> | ||
sdumitriu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| <properties> | ||
| <aspectj.version>1.9.2</aspectj.version> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
sdumitriu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| </properties> | ||
|
|
||
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
sdumitriu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| <artifactId>maven-compiler-plugin</artifactId> | ||
| <configuration> | ||
| <source>1.8</source> | ||
| <target>1.8</target> | ||
| </configuration> | ||
| <version>3.8.0</version> | ||
| </plugin> | ||
|
|
||
| <plugin> | ||
| <groupId>io.qameta.allure</groupId> | ||
| <artifactId>allure-maven</artifactId> | ||
| <version>2.10.0</version> | ||
| </plugin> | ||
|
|
||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-surefire-plugin</artifactId> | ||
| <version>3.0.0-M3</version> | ||
| <configuration> | ||
|
|
||
| <argLine> | ||
| -javaagent:"${settings.localRepository}/org/aspectj/aspectjweaver/${aspectj.version}/aspectjweaver-${aspectj.version}.jar" | ||
| </argLine> | ||
|
|
||
| <suiteXmlFiles> | ||
| <!-- Specify POM file through command line using -Dsurefire.suiteXmlFiles=... | ||
| Default to not run any tests so that doesn't affect mvn build of phenomecentral.org --> | ||
| <!-- <suiteXmlFile>src/test/java/org/phenotips/endtoendtests/testcases/xml/AllTests.xml</suiteXmlFile> --> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above, the default should be |
||
| <suiteXmlFile>src/test/java/org/phenotips/endtoendtests/testcases/xml/NoTests.xml</suiteXmlFile> | ||
| </suiteXmlFiles> | ||
| </configuration> | ||
|
|
||
| <dependencies> | ||
| <dependency> | ||
| <groupId>org.aspectj</groupId> | ||
| <artifactId>aspectjweaver</artifactId> | ||
| <version>${aspectj.version}</version> | ||
| </dependency> | ||
| </dependencies> | ||
| </plugin> | ||
|
|
||
| </plugins> | ||
| </build> | ||
|
|
||
| <dependencies> | ||
|
|
||
| <dependency> | ||
| <groupId>org.seleniumhq.selenium</groupId> | ||
| <artifactId>selenium-java</artifactId> | ||
| <version>3.141.59</version> | ||
| </dependency> | ||
|
|
||
| <!-- Let's use TestNG instead of JUnit--> | ||
| <dependency> | ||
| <groupId>org.testng</groupId> | ||
| <artifactId>testng</artifactId> | ||
| <version>6.8</version> | ||
| </dependency> | ||
|
|
||
| <!-- https://mvnrepository.com/artifact/commons-io/commons-io --> | ||
| <dependency> | ||
| <groupId>commons-io</groupId> | ||
| <artifactId>commons-io</artifactId> | ||
| <version>2.6</version> | ||
| </dependency> | ||
|
|
||
| <!-- https://mvnrepository.com/artifact/io.github.bonigarcia/webdrivermanager --> | ||
| <dependency> | ||
| <groupId>io.github.bonigarcia</groupId> | ||
| <artifactId>webdrivermanager</artifactId> | ||
| <version>3.3.0</version> | ||
| </dependency> | ||
|
|
||
| <!-- https://mvnrepository.com/artifact/io.qameta.allure/allure-testng --> | ||
| <dependency> | ||
| <groupId>io.qameta.allure</groupId> | ||
| <artifactId>allure-testng</artifactId> | ||
| <version>2.10.0</version> | ||
| </dependency> | ||
|
|
||
| </dependencies> | ||
|
|
||
| </project> | ||
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.
This shouldn't be recommended, the code shouldn't be changed just for adapting to the environment. That's what environment variables are for.