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

PC-325 Integration Tests with squished commits #113

Open
wants to merge 37 commits into
base: master
Choose a base branch
from

Conversation

BrianLiSK
Copy link

@BrianLiSK BrianLiSK commented Mar 4, 2019

This is a PR for PC-325 (Integration Tests) from my fork of the project with master of the official phenomecentral.org. https://github.com/BrianLiSK/phenomecentral.org/tree/PC-325_Squashed

The original (now archived) Integration Tests standalone project is here: https://github.com/BrianLiSK/PhenomeCentralAutomation

The commits were squashed down to a more reasonable 37 commits and then merged to the end-to-end-tests subdirectory. This was done instead of adding the project as a git submodule, which might complicate things a bit more if you don't have access to the original project.

The overall commit date reflects the merge date, however, the original authoring timestamp was preserved. Perhaps I can do a merge to master of my fork of phenomcentral.org first to see what happens.

I auto-formatted all the code using the rules provided to intelliJ which seemed to add a lot of white space, hence the inflated line count.

I need to add a AGPL license to each file, so I will address that along with any other comments you will certainly have.

Thanks. I know it is a large PR.

- Initial commit. Will move to main patient-network project when ready. This is pretty seperate anyways. Can run JSON import and login tests. Still much to do.
[PC-325] Added a comment to describe the purpose of each class. Added an interface for common selectors on a patient info page. This contains the possible sections one can see on either the View or Create patient page.

[PC-325] Import JSON test now asserts that the several sections that should be visible after consent is enabled. Also rearranged code a bit to use enums in the test case.

Added checking the admin page for matches after the patient is created via JSON. Run createPatientTest class to see.

Updated readme. Made test send emails to matched users.

[PC-325] Modified importJSON test to import two patients at once and to notify users via email. Added a testNG xml file called multipledClasses.xml as a demo for multiple classes of test for a suite.
This will probably be a needed dependency.
comments

Reformatted the code based on provided style xml for intelliJ. Going to clean up code some more before continuing.

Added comments and code style to basePage class.

Renamed basePage class to BasePage

Added comments to commonInfoSelectors.

Renamed class commonInfoSelectors to CommonInfoSelectors

[PC-325] Eliminating dead/commented out code.

[PC-325] Added comments to adminMatchNotificationPage and a seperate class for the MockMock's email UI homepage.

[PC-325] Renamed adminMatchNotificationPage to AdminMatchNotificationPage

[PC-325] Made the adminMatchNotificationPage a subclass of AdminSettingsPage. Added comments there too.

[PC-325] Renamed adminSettingsPage => AdminSettingsPage

Added comments to the allPatientsPage

[PC-325] Added comments to several more classes.
[PC-325] Renaming several classes to start with a capital letter.

[PC-325] Renamed packages to begin with a capital letter. This mirrors the style within patient-network.

[PC-325] Tidied up demo tests with comments and broke up the tests into smaller methods.
PC-325: Improved verifyEmailNotifications() test by actually checking that there are two emails there.

PC-325 Fix compilation issue, put java 1.8 as a build dependency in the pom file.

PC-325: Adding methods and selectors to allow for manual patient creation. Will continue tomorrow.

PC-375: Adding a test to create a basic patient with phenotypes/genotypes manually. Still working on ensuring test is stable enough for different browser window sizes. For some reason, selenium isn't scrolling some elements to the viewport on Firefox. Need to investigate some more.

PC-325: Expanded create patient test case. Investigating why selenium doesn't automatically scroll. Probably going to do a dpeendency update.

PC-375: Updated dependency of Selenium to 3.1415. Debugged dropdown not actually selecting due to missed click and wrong selector. CreatePatientTest should now work properly. Going to double check.
creation tests

PC-375: Added a helper method to delete all patients from the table on admin view.

PC-375: Added selectors and methods for modifying visibility and patient permissions.

PC-375: Added a publci visibility test. Currently conflicts with the verifyEmailNotifications() test. Probably just need to move to a new class. Will ask about matches not showing up fast enough.

PC-375: Added annotations to ignore deletion of created users. Mdoularized each test to run independently or if needed, with the dependsOnMethods annotation.

PC-375: Added test for collaboration visibility. Asserts that the added user will be able to see the specified user.

PC-375: Changed currentPage variables to something more descriptive. Code refactoring.

PC-325: Added methods to navigate to the Admin's "Refresh matches" page. This is used in the test refreshMatchesForTwoPatients() which goes into the refresh matches page, updates the matches, and asserts that two new ones are found. The assertion will fail currently because matches require a server restart to appear.

PC-325: Added method to remove a collaborator from a patient. removeNthCollaborator() is used in the test to reset to a previous state.

PC-325: Assigned priorities to each test so that they run in order as needed.
PC-325: Added a test to cycle through the options for creating a patient. Will continue tomorrow.

PC-325: Pre-order traversal of the age of onset buttons added. Added method to allow for the Indication For Referral box to be set. Improved the cycling test and added some more comments.

PC-375: Fixed the publicVisibilityTest and made the page for Refresh Matches actually refresh before checking values within the table.

PC-325: Helper function for a pre-order traversal. Realized that the traverse is only finding the radio button so it cannot populate an array of Strings to represent the labels of each button. Have to do some more digging about how the tree is traversed. Hence there is commented out code in preOrderTraverseAndClick(). The end goal is to have it return the labels of the nodes visited so that the List can be asserted for correctness during a test. Will continue tomorrow.

PC-325: Actually, there maybe nothing wrong with the traverse function. I just passed the wrong child label path to it. Seems to work now. Uncommented the code in the preOrderTraverseAndClick and just changed the child selector path where I concatenated a "> label".

PC-325: Added methods to modify the date of death on the create patient page. Made the cycling test assert that the array of labels is equal to what is expected to be there during a pre-order traversal.
PC-325: Added methods to navigate to the Pedigree editor from the create patient page. Added method to handle the save warning dialogue box when trying to close the editor.

Pc-325: Added methods for retrieving basic data that is present within the pedigree editor. Ex. Gender of patient. Will need to add more methods to interact with individuals within the editor.

PC-325: Added more methods to get the entered patient info from the pedigree editor.

PC-325: Got the click on the patient edit modal to finally work using an Action class. Need to do some more reading on it as SVGs apparently aren't directly interactable as I thought. Also created a new class for the pedigree page tests, it currently contains a basic one for me to check if navigation to editor works so far.

PC-325: Added method to link to a patient via the pedigree editor. Still working on tree traversal logic. Not working yet. Push this for now.

PC-325: Added methods to retrieve the number of various nodes within the tree. Created a simple test for creating a child when there is one patient present. Need to make methods more robust, abstract away hovering to helper functions, and clean up some commented code.

PC-325: Added methods to create sibling and one for partnership. Selectors are still very unstable. Need to work out why that is, js is causing signficant changes to make it difficult to locate the one selector that is appropriate. Trying to think about logic to make it work. Will have to continue on Monday.
PC-325: Added traversals for the prenatal section.
- Changed tree traversal method to return null whenever an invalid condition is found and message to stdout. This will usually be because of invalid/mismatching selectors passed to traversals. Also added in a JS click that I'll abstract out.
- Need to clean up hardcoded selectors, making them a variable, still debugging a bit
- Added test case for tree in prenatal conditions

PC-325: Removed dead code. Cleaned up selectors a bit. Ensured assertion array for prenatal conditions contained all conditions. Test now passes.

PC-325: Added ethnicity setters to patient creation page. Moved the arrays of yes/no labels used in assertion to its corresponding individual test method (previously, they were private in class scope).

PC-325: Added test to check label contents of phenotype details box. Does not do tree a full tree traversal as expansion seems to be broken. Added method to add phenotype detail to Nth phenotype.

PC-325: Helper function for scrolling with JS and then click. Avoids repeat try/catch blocks. For some reason Selenium does not scroll automatically in some cases causing elements to go out of its viewport. Investigating.

PC-325: Traversal for phenotype tree attempt. Sometimes, does only one level for some reason, seems to be a loading issue. Phenotypes tree take a while to traverse and will have a very long list.

PC-325: Added priorities for the tests on create patient options.
PC-325: Test cases for comparison between data on Pedigree Editor and data from Patient Form
- Added methods to extract phenotype information from the create patient form
- Added methods to add phenotype and genotype information to patient via pedigree editor
- Changed rectangle selector for siblings to just grep for the size portion of the transform matrix. That selector returns all rectangles of that type. Can then determine which one by 2n-1... hopefully...

PC-325: Slight modification of pedigree editor tests. Removed redundant steps.
- Added in an unconditional wait as for the "loading pedigree" modal after creating a patient doesn't fully disappear even though the built in wait has decided that the element is then clickable.

PC-325 Refactored some code.
- Helper methods to add a list of phenotypes
- Method that toggles the first four unchecked consent boxes.
- These help shorten test case writing a bit.

PC-325 Added methods to retrieve genotype information from patient form.
- Improved a pedigree editor test case to compare genotype edits to what appears on the patient page.

PC-325: Added priorities to test cases for the pedigree page. Investigating minor stability issue with tests where clicks within the svg start failing.
- Pedigree editor stability fix

PC-325 Added a class to be used as a struct for a patient's measurements.

PC-325: Added method to input a new measurement entry to patient form.
- Added a wip test case.
- Updated the native clickOnElement selector to force a scroll if a click fails.

PC-325 Fixed hover over issue followed by click on pedigree editor.
- Turns out unresponsive click was due to not waiting until the listener activated on hover over. Much simpler than I thought.
- Added methods to retrieve the first measurement entry from a patient.

PC-325: Overrode toString() and equality comparison methods in the PatientMeasurment class. These are used in assertions.
- Measurements test case now asserts the data on the saved entry.
- Added method in BasePage to explicitly wait for an element to disappear.
- Added support to specify arbitrary date for measurement on Calendar.
- Added method to retrieve measurement entry. Need to improve to work for Nth entry.

PC-325 Added test case for checking phenotypes automatically added by measurement entry.
- Automatically added phenotypes have lightning bolts so the new selectors just check/filter for that
- Added methods to get phenotypes that were added in different ways (auto via mesurements, manually added, all)
- Modified existing tests to call the get all phenotypes method

PC-325 Improved stability of pedigree tests.
- Added fail-safe mechanism to pedigree editor modals, tries to re-open modal when a first attempt fails.
- Above might happen when modal is accidentally closed from a previous operation.
- Fixed typo "Confirmed Causal -> Confirmed causal"
- Slight modification of input traversals in other sections

PC-325 Inserted additional label in inheritance traversal.
- This is a missing label that needed to be asserted for during the test. See PT-3935

PC-325 Added remaining boxes within Prenatal History section in the traversal test.
These boxes are clicked and text inputted.

PC-325 Added methods to interact with the Diagnosis section on the Patient Form.
- Methods and selectors for each input box within the Diagnosis section
- Small test case for checking all input boxes. Will continue on with basic error checking.

PC-325 Inserted more assertions for the visibility of the PubMed boxes when the Case Solved checkbox is toggled.

PC-325 Modified the method that checks for visiblity of the PubMed boxes.
- Changed so that it checks for clickability as the hidden property is still considered visible to the automation framework.
- Added a test case to check for the error messages upon invalid PubMed ID input.

PC-325 Added test for diagnosis section on patient form.
- Methods to extract information from diagnosis section from View Patient page.
- Check error messages upon entering invalid pubMed ID
priorities

PC-325 Use only priorities for tests, do not mix with dependsOnMethods as that overrides the priority field.

PC-325 Grouped methods and selectors toegether according to sections. Added comments to indicate which section.
PC-325 Added methods for the creation of users via the admin panel
- Two new pageobject classes: One for the Users page and the other for the Admin Pending Users page.
- Small wip test cases for the adding and approving users.
- Methods to navigate through menu options in the Admin Settings page.

PC-325 Modified matching test to assert for the numbers as Strings
- Test were failing due to numbers not being treated as Strings
- Solr fix seems to be working, matches are updated live
- Removed unused variables from test

PC-325 Added methods to notify two specific patients who appear in the match table.
- Added a temporary test for emailing users
- Added a methods to toggle the contacted/uncontacted status

PC-325 Moved Signup fields selectors to an interface as these appear on both the admin users page and the user sign up page from the homepage.

PC-325 Added test case for ensuring unapproved user sees the Pending Approval message on various pages.
- Methods to retrieve the confirmation as well as the pending approval messages
- New class for the User Sign Up page
- The additional test case for the unapproved user, more to follow

PC-325 Added tests for when user is unapproved to ensure they cannot do anything.
- Similar test for when user gets approved
- Added method to navigate back to homepage by clicking on the PC logo on the toolbar.

PC-325 Added test to error check the input fields of the sign up (Request Account) page.
- Two new methods in the BasePage class to clear an element of text (ex. Text box) and to toggle a checkbox to checked state (instead of toggle regardless)
- Requesting an account method now clears the "username" field so that it can be regenerated as it inputs text to other fields.
PC-325 Added test cases for matching with only genotype and one with only phenotype.
- Overrode navigateToHomePage on the EmailUIPage to directly navigate to the homepage  URL rather than click on PC logo on top left

PC-325 Changed the gene in matchPhenotypes only so that all genotypes are different in that case.
- Added priorities to each test so that they run in a deterministic order.

PC-325: Added methods to move average score and genotype score sliders to their minimum values (Slide all the way to the left)
- Incorporated the genotype slider to slide all the way to the left for phenotype only match test

PC-325 Method that returns a bool to whether or not a match for a given two patients appear on the match table.
- Test case for a user to be presented with error page when trying to access another user's matchable patient
PC-325 Added an xml for All Test cases. Making the driverstatic so that it does not open all those windows at once.

PC-325 Made the instance of WebDriver static so that only one browser window is used. Keeping it as protected instead of making private as I am lazy to add an accessor.
PC-325 Last commit for Friday. Simplified AllTests.xml. WIP of screenshot mechanism.
- Currently running all the tests, one driver instance/browser window.

PC-325 Screenshot on test failure (whether assertion or exception) working.
- Screenshots do not have URL of the page, investigating on ways to insert it.
- Added dependency `FileUtils` to pom as we need to copy over the file to target/screenshot directory. Messy without extra dependency.

PC-325 Removed dead code in the BaseTest class.
- No longer pause before quitting the webdriver, before, pause was to manually see the failure as we watch the automation run.
- Added comments to AfterClass and AfterSuite methods.

PC-325 Refactored addGene() method to allow for strategy to be specified.
- Method will now tick off a specified gene strategy tickbox.
PC-325 Refactored PedigreeEditorPage.
- Added comments sectioning off the selectors/methods to different areas of the page.
- Added selectors for the alive/deceased status radio buttons on the patient info modal
	- Will create methods to interact with them in the future

PC-325 Additional test for pedigree editor warning dialogue
- Test asserts that the warning dialogue appears when trying to navigate away from editor after data changed.
- Added a wait for the three save option buttons on the warning dialogue, will throw exception if any of those aren't there

PC-325 Did a `git add .` too soon. Missed two lines from previous commit.
- Minor change to the test case, change a gene instead of phenotype.
- This class sets up the instance for the test run by creating the two desired users and setting email port to MockMock's default of 1025.
	- This is useful when running automation using a local instance. MockMock's email UI is a bit fancier than the one on VMs as we can extract titles of emails easily (what the test is currently checking for). In the future, need to ensure the setup works for VMs.
PC-325 Minor test case bug fixes.
- Ensured email port is set first before adding users.
- Navigation to Mail Settings page now uses correct selector

PC-325 Removed priorities to each test as they are global vars and having same ones cause conflict
- Rewrote xml to include all methods explicitly so that the desired order is defined there
- Priorities cause class order in xml to be ignored despite `group-by-instance="true"` as priority takes precedence
	- Having multiple priorites of the same value across classes is not recommended, priorities are global vars

PC-325 Small test case fix. Need to tell test to navigate to the home page explicitly after logging in. This is done before checking the splash page's integrity.

PC-325 Added an assertion for the HPO tree. It is a very long array, so maybe I'll take it back out in the future.
- We really should assert something at least. It does the first level depth.
- Some refactoring of wait times

PC-325 Added support for different browsers.
- Ctor of abstract BaseTest now sets up environment for the desired browser
- Specify desired browser by modifying activeBrowser variable as desired
- Added WebDriverManger dependency to maven pom. This is a library that does all the nitty gritty environment setup such as automatically locating the path to the browser executable.

PC-325 Moved xmls into their own package (as subdir of test cases classes)
- Fixed a bug where static webdriver wasn't being checked and a new instance was always instatiated for each class.

PC-325 Refactored code to avoid a wait
- This was a source of signficant delay. We do not want to find the log out link first.
	- Instead, we try to click on the login button and only then do we try to log out.
	- This caused a three second delay everytime we tried to navigate to login page.

PC-325 Added a common method to wait for loading bar to disappear.
- In process of implementing that in various places instead of unconditional wait
- Also allow unconditional wait to be passed a long instead of just an integer
… names

PC-325 Renamed package PageObjects to pageobjects (lowercase)

PC-325 Renamed package "TestCases" to "testcases"

PC-325 Moved all code under org.phenotips directory
- Bug fix for PT 1.4.4, handle unsaved changes warning dialogue

PC-325: Commented out ALl_Patients_URL main variable. Should work without it.

PC-325 Added maven surefire plugin as a dependency.
- This allows one to specify an xml to run via TestNG via the command line

PC-325 Pulling out unconditionalWaits as much as possible from all classes.
- This is wip and might not run properly. Committing for now as it is end of day.
- Wrote new methods to wait for the spinning waiting message that can appear at the bottom of the page during an ajax request to the server.

PC-325 Commented on the remaining unconditionalWaits explaining why they are needed.
- Sometimes we really don't have anything to wait on, there is a background ajax request that needs to get completed ex. username generation
- Tests have not been run yet, will see if any additional ironing out is needed.

PC-325: Ensured that each test logOut() after each run.
- Added some very messy code to handle (dismiss) the warning dialogue that can popup.
- We have a fire right now (PT-3952). I could potentially have a bad build so maybe thats why the dialogue is appearing.

PC-325 Added a method that dismisses the unsaved changes warning before navigating away from patient editor page.
- We explicitly dismiss that modal when trying to navigate away from patient form without saving
- UnconditionWait placed back into pedigree editor. Wait 5 seconds for now. Easy fix. Proper fix would be to wait until the "Patients not in pedigree" loads

PC-325 Changed the noMatchPrivatePatient test to use different genotype than genoOnlyMatch test.
- Had a weird behaviour where private patient was matching to the genotype of the genoOnlyMatch test. Possibly timing issue as we refreshed immediately after creating the patient.
(Squashed commits)

PC-345 Allow port numbers and browser to be specified through command line
- Three optional parameters that can be passed in command line as SystemProperties to JVM
	- "browser", "homePageURL", and "emailUIPageURL"
	- browser can be one of: "chrome", "firefox", "edge", "ie", "safari"
	- Example usage: -Dbrowser="firefox" -DhomePageURL="localhost:8083"
- HOMEPAGE_URL and EMAILUI_URL are no longer final variables but have default value
	- If environment variable was specified, then it will mutate them to desired value
	- Having default values in assignment allows for running tests on IDE without changing JVM parameters in settings. To change PC instance location, you can paste in the desired location to the variable rather than have to adjust the IDEs settings for JVM parameters
- A debug message is printed out when any of the three command line paremeters are specified/passed. Printed out once during webdriver initalization.

PC-345 Allow port numbers and browser to be specified through command line
- Three parameters that can be passed in command line as SystemProperties
	- "browser", "homePageURL", and "emailUIPageURL"
- HOMEPAGE_URL and EMAILUI_URL are no longer final variables but have default value
	- If environment variable was specified, then it will mutate them to desired value
	- Having default values in assignment allows for running tests on IDE without changing JVM parameters in settings. To change PC instance location, you can paste in the desired location to the variable rather than have to adjust the IDEs settings for JVM parameters
- A debug message is printed out when any of the three command line paremeters are specified/passed. Printed out once during webdriver initalization.
- Initial implementation

PC-347 Very wip script. Do not use. Pushing before weekend.

PC-347: Write a script to extract, startup instance and run tests
- Added methods to start both server and SMTP

PC-347: Add script to extract, start and run e2e tests.
- Some error fixes for using variables (removed quotation marks to allow
globbing patterns)

PC-347 Script to extract, start and run e2e tests.
- Forgot to save some changes from previous commit. Adding them now.
Mostly rearrangement of code.

PC-347 Script to extract, start and run e2e tests
- Added some comments to script

PC-347 Script to extract, start, and run e2e tests
- Moved script and SMTP jar to scripts folder

PC-347 Script to extract, start, and run e2e tests
- Corrected relative path to distribution folder

PC-347 Script to extract, startup, and run e2e tests.
- Bug fixes regarding the ports to be used by default
- Added ctrl-c capture mechanism that kills the SMTP and PC instance upon exit
- Removed old code.

PC-347 Script to extract, startup, and run e2e tests
- Added support for one command line arg to support browser, good for a demo for now
- Added parameters for maven command as variables

PC-347 Script to extract, start, and run e2e tests.
- Waits 30 seconds after running start before trying to ping with curl
- Specify http in URL as firefox does not recognize protocol without it.
PC-346 Implement Allure reporting.
- Created an xml containing two tests, one that passes and other one doesn't
- Basic allure reporting coming together. JSONs are generated for a report generation.

PC-346 Integrate Allure reporting
- JSON for reports are now generated in target/allure-reports folder instead of root directory
- Screenshot attachments appear on Allure report

PC-346 Add Allure reporting
- Allure report is now generated after the e2e tests are run. This is a second maven command.
- Additional dependency to allow report generation

PC-346 Add Allure Reporting.
- Added @step annoations to pageobject code so that steps are visible from within the report
subdirectory

[Misc] Moved test to their proper subdirectory of endtoendtests (after phenotips package)

[Misc] Moved files to their subdirectory of endtoendtests (after org/phenotips)
generation of Allure reports (Squashed Commits)

PC-346 Added comments to script. Commit for end of the day.

PC-347 Changed the name of the log file and associated .gitignore.
- Will work on cleanup of tee command

PC-347 Startup script - Moved tees to an exect command
- The two exec commands now capture the entire scripts output rather than having the `tee` command littered everywhere

PC-347 Startup script - Changed date format of created folder
- Grouped more code in their own function

PC-347 Startup script
- Script now creates an instances/ folder under scripts where the logs and PC instance extracts go
- Made git ignore the instances folder

PC-347 Removed the demo script that ran integration tests

PC-347 Startup script
- Fixed pom dependencies to use latest aspectj version and to provide version for the maven compiler plugin
- Moved SMTP service to its own subdirectory in scripts. This will be the SMTP service the script starts up before starting the PC instance
- Added brief Readme file for the SMTP service linking to original github page for MockMock
- Removed old demo runIntegrationTestsDEMO.sh (previous commit?)

[Misc] Renamed src/main folder to src/test folder
- Adjusted path of TestNG XML

PC-347: Missed changing this line back to src/test folder as directory was renamed in prev commit.

PC-346, Misc - Refactor BaseTest code.
- We now pass URL information about the page of the failure. This appears on the Tear Down section onTestEnd() method on Allure Report
- Divided aftermethod to multiple helper functions for better readability.

PC-347 Startup script
- Refactor vars to upper case letters
- Ensured consistency in regards to forward slashes for directories

PC-347 Fixed argument par
[Misc] Specified UTF-8 encoding for POM file
- Specify character encoding on POM file to prevent maven warning
- Added comments to BaseTest class

[PC-325, Misc] Cleanup of CreatePatientPage
- We should not handle invalid input from info passed in from a test case. If the desired dropdown, date, or option is not found, we should just fail the test as Selenium will throw ElementNotFound exception rather than handling that error in the framework gracefully
- Duplicate selectors, that are exactly the same, have been moved to a seperate variable

[Misc] Refactor CreatePatientPage and Update PatientCreationOptions class
- Updated assertion for the HPO list on 1.4.4 (slightly different than what it was on 1.4.2)

[Misc] POM file update
- Specify no tests by default to prevent it from running during the default `mvn test` or `mvn install` of parent project

[Misc] POM update to phenomecentral group ID
commits)
- Various refactoring including line spacing changes

[Misc] Refactor CreatePatientTest class, incl spacing.

[Misc] Reorganized BasePage class. Ensured that similar methods are grouped together to be more readable.

[Misc] Refactored BasePage comments - minor typo.

[Misc] Reformat AdminMatchNotificationPage code

[Misc] Reformatted AdminEmailSendingSettingsPage

[Misc] Reformatted AdminPendingUsersPage (auto reformat spacing)

PC-325 Added a small invalidCredentials test case and refactored some code.

[Misc] Reformat all code using rules provided to intellij (spacing).

[Misc] Moved common classes used in both testcases and pageobjects to its own package called "common"
- Edited some comments, small bug fix

PC-346 Instances folder now extracts to the target/ directory.

[Misc] Multiple typo addressing in the comments, bug fix for refresh.
- Refresh matches should happen before the refreshMatchesForTwoPatients() is run. This will ensure that the number of patients after refresh since last modified will be correct.
- Much more information added
- New features such as Allure and detailed usage information

PC-325 Readme update
- Updating readme with updated information about dependencies and usage
- Commit author should change.

[Misc] Readme update

[Misc] Readme update

[Misc] Update readme
- More detail, a bit wordy still

[Misc] Added license information to distributed smtp jar
- Apache 2.0 License. Distribution and private use allowed.
- Moved all files into end-to-end subdirectory
subdirectory
Merged into end-to-end-tests folder of this project
Merge remote-tracking branch 'temp/amendingTest' into PC-325_Squashed
Copy link
Member

@sdumitriu sdumitriu left a comment

Choose a reason for hiding this comment

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

First batch of comments, more to follow.

end-to-end-tests/phenomeCentralautomation.iml Show resolved Hide resolved
- 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
Copy link
Member

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.

- 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.
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Author

Choose a reason for hiding this comment

The 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 phenomecentral.org with the default mvn install command would have a browser pop up for ~30 mins while it runs these e-2-e tests. Although they can minimise the window, it would still draw resources and one can accidentally interfere with the tests by clicking or typing into the window.

- 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.
- 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.
Copy link
Member

Choose a reason for hiding this comment

The 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:

  • not depend on other tests being executed before
  • not be broken if another test did run before

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.

Copy link
Author

Choose a reason for hiding this comment

The 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.

end-to-end-tests/pom.xml Show resolved Hide resolved

private final By maternalEthnicityBox = By.id("PhenoTips.PatientClass_0_maternal_ethnicity_2");

private final By addEthnicityBtns = By.cssSelector("div.family-info a[title=add]");
Copy link
Member

Choose a reason for hiding this comment

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

This seems unused.

The selector isn't very stable, other lists may be added in the future in this section. At least use .fieldset.ethnicity a[title=add], which will only look at ethnicity add buttons. And as I said already, I don't like hardcoding translatable text in selectors, but at the moment the button lacks a better way to identify it.

public CreatePatientPage setIdentifer(String identifer)
{
clickOnElement(identifierBox);
superDriver.findElement(identifierBox).clear();
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't clickAndTypeOnElement clear too?

waitForElementToBePresent(lifeStatusDrp);
Select statusDrp = new Select(superDriver.findElement(lifeStatusDrp));

statusDrp.selectByVisibleText(status);
Copy link
Member

Choose a reason for hiding this comment

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

selectByValue is independent of translations, but on the other hand selectByVisibleText is what users do. I guess we could write a test where we check that setting Fallecido works and then shows up as Deceased when switching back to English.

{
forceScrollToElement(sectionMap.get(theSection));

clickOnElement(sectionMap.get(theSection));
Copy link
Member

Choose a reason for hiding this comment

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

Clicking toggles, either change the method name to toggleSection, or add a check: if (driver.findElement(sectionMap.get(theSection)).findElement(By.xpath("parent::*[contains(@class, 'collapsed')]") != null) { click on it }

* @param theSection is the section from the enum that we want to expand
* @return stay on the same page so the same object
*/
public CreatePatientPage expandSection(SECTIONS theSection)
Copy link
Member

Choose a reason for hiding this comment

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

This method is out of place among other methods for interacting with the patient information section. Move it somewhere else.

aLoginPage.loginAs("", "123"); // Empty username
aLoginPage.loginAs("SomeoneLikeYou", ""); // Empty password

aLoginPage.loginAsAdmin().logOut();
Copy link
Member

Choose a reason for hiding this comment

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

This seems out of place.

@Test()
public void cycleThroughAllPhenotypes()
{
final List<String> checkPhenotypeLabels = new ArrayList<>(Arrays.asList(
Copy link
Member

Choose a reason for hiding this comment

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

This looks very ugly. HPO changes a lot, so tests like this will become broken very soon. Thus, I don't think it's needed.

What would be more useful is a check that certain key elements are present: there's Decreased body weight (<-2SD) under Growth Parameters -> Weight for age, there's Cleft palate under Craniofacial, etc.

@Step("Dismiss the unsaved changes warning dialogue")
public void dismissUnsavedChangesWarning()
{
superDriver.switchTo().alert().accept();
Copy link
Member

Choose a reason for hiding this comment

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

Since this method is called from many places, you should note that it may fail, since autosave may happen before you try to navigate away and there won't be any unsaved changes to notify about. Better put it in a try/catch.


Assert.assertEquals(aHomePage.getUnauthorizedErrorMessage(), unauthorizedActionMsgCheck);

anAdminRefreshMatchesPage.logOut();
Copy link
Member

Choose a reason for hiding this comment

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

anAdminRefreshMatchesPage is not the right one to use.

@sdumitriu
Copy link
Member

Running the tests fails:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.0.0-M3:test (default-test) on project phenomeCentral-automation: There are test failures.
[ERROR] 
[ERROR] Please refer to /home/sg/Work/PhenomeCentral/end-to-end-tests/target/instances/PCInstance_2019-03-29_10-35-23/phenomecentral-standalone-1.3-SNAPSHOT/../../../../target/surefire-reports for the individual test results.
[ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
[ERROR] There was an error in the forked process
[ERROR] 
[ERROR] Cannot instantiate class org.phenotips.endtoendtests.testcases.CreatePatientTest
[ERROR] org.apache.maven.surefire.booter.SurefireBooterForkException: There was an error in the forked process
[ERROR]
[ERROR] Cannot instantiate class org.phenotips.endtoendtests.testcases.CreatePatientTest
[ERROR]         at org.apache.maven.plugin.surefire.booterclient.ForkStarter.fork(ForkStarter.java:657)
[ERROR]         at org.apache.maven.plugin.surefire.booterclient.ForkStarter.run(ForkStarter.java:283)
[ERROR]         at org.apache.maven.plugin.surefire.booterclient.ForkStarter.run(ForkStarter.java:246)
[ERROR]         at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeProvider(AbstractSurefireMojo.java:1161)
[ERROR]         at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeAfterPreconditionsChecked(AbstractSurefireMojo.java:1002)
[ERROR]         at org.apache.maven.plugin.surefire.AbstractSurefireMojo.execute(AbstractSurefireMojo.java:848)
[ERROR]         at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:137)
[ERROR]         at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:210)
[ERROR]         at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:156)
[ERROR]         at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:148)
[ERROR]         at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:117)
[ERROR]         at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:81)
[ERROR]         at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:56)
[ERROR]         at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128)
[ERROR]         at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:305)
[ERROR]         at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:192)
[ERROR]         at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:105)
[ERROR]         at org.apache.maven.cli.MavenCli.execute(MavenCli.java:956)
[ERROR]         at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:288)
[ERROR]         at org.apache.maven.cli.MavenCli.main(MavenCli.java:192)
[ERROR]         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[ERROR]         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[ERROR]         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[ERROR]         at java.lang.reflect.Method.invoke(Method.java:498)
[ERROR]         at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
[ERROR]         at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
[ERROR]         at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
[ERROR]         at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)

@sdumitriu
Copy link
Member

More details:

Caused by: org.openqa.selenium.SessionNotCreatedException: session not created: This version of ChromeDriver only supports Chrome version 74
  (Driver info: chromedriver=74.0.3729.6 (255758eccf3d244491b8a1317aa76e1ce10d57e9-refs/branch-heads/3729@{#29}),platform=Linux 4.14.8-gentoo-r1.efi x86_64) (WARNING: The server did not provide any stacktrace information)

I'll upgrade my Chrome and try again.

@sdumitriu
Copy link
Member

image

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.

4 participants