sardell opened a new pull request #1317: Metron 1958 URL: https://github.com/apache/metron/pull/1317 ## Contributor Comments Link to the original ASF JIRA ticket: https://jira.apache.org/jira/browse/METRON-1958 As described in the ticket, we have a few anti-patterns in place right now in regard to our Cypress configuration. I believe this has led to the recent Travis failures in Pull Requests like #1308 and #1275. ### Changes Included * Update npm scripts to use the express server that's already in place vs. the angular cli development server. Lately, we have been running into a race condition where sometimes the Cypress tests are running before the application has been compiled and the server has started. We should be testing the built application after the build is complete. * Add a baseUrl property to the Cypress config [per their recommendation](https://docs.cypress.io/guides/references/best-practices.html#Setting-a-global-baseUrl). * Remove the logout click action in the afterEach hook [per the Cypress team's recommendation](https://docs.cypress.io/guides/references/best-practices.html#Using-after-or-afterEach-hooks). While we had to do this to clean up state when the tests were run in protractor, it's unnecessary in Cypress and can cause errors if a test is reloaded before the afterEach executes or loaded before the page reload is complete. * Add yamljs as a dependency in the alerts ui. This dependency is already part of the project in the config ui and was already used by the express server to parse the configuration file. * Update the Cypress tests to use the updated global configuration endpoint. ### Testing Clone either #1308 and #1275 locally. From the command line, navigate to metron-interface/metron-alerts from the root of the project. Install npm dependencies by running `npm ci`. Then, run `npm run cypress:ci`. #1308 will always fail, and #1275 will intermittently fail. Once you've verified failure on either on of the PRs mentioned above, clone this PR locally. Merge it into #1308 or #1275, run `npm ci`, then run `npm run cypress:ci`. The tests should consistently pass now. ### Other Considerations The Cypress team recommends the use of the `wait-on` or `start-server-and-test` modules to handle the flow of starting your server, starting Cypress and, once the tests complete, killing your server. However, since the next proposed Metron release is near and this PR will fix failing PRs needed for that release, I decided instead to continue using the npm-run-all module we were already using. Since the express server is so incredibly fast compared to the startup of Cypress, I felt the risk of a race condition happening would be extremely low and this was the best approach for now. I've created a ticket around the addition of the recommended modules to keep track of the task: https://jira.apache.org/jira/browse/METRON-1959 In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel). - [ ] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)? ### For code changes: - [ ] Have you included steps to reproduce the behavior or problem that is being changed or addressed? - [ ] Have you included steps or a guide to how the change may be verified and tested manually? - [ ] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via: ``` mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh ``` - [ ] Have you written or updated unit tests and or integration tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [ ] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent? ### For documentation related changes: - [ ] Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via `site-book/target/site/index.html`: ``` cd site-book mvn site ``` #### Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. It is also recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
