METRON-1958: Optimize Cypress to use best practices

[sardell]

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. Through local testing, 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.
• Remove the logout click action in the afterEach hook per the Cypress team's recommendation. 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.
• 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?

• 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 is set up for your personal repository such that your branches are built there before submitting a pull request.

[merrimanr]

Has this been tested in full dev? The alerts_ui.yml file is under Ambari control and I don't see that file updated in this PR. I think you may need to add the dirPath property to that template at https://github.com/apache/metron/blob/master/metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/templates/alerts_ui.yml.j2 or add a default value in the alerts-server.js script.

I am going to test in full dev now and will report back.

[merrimanr]

I confirmed this in full dev. The Alerts UI fails to start unless you add dirPath: alerts-ui to https://github.com/apache/metron/blob/master/metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/package/templates/alerts_ui.yml.j2.

[merrimanr]

I opened a PR with the fix: sardell#16.

[sardell]

@merrimanr Thanks for catching that and opening a PR with the fix. I just merged it into my branch.

[merrimanr]

Thanks @sardell, this looks good to me. +1

[mmiklavc]

I have a post-merge comment - I really like this change:

Remove the logout click action in the afterEach hook per the Cypress team's recommendation. 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.

I've used this pattern for integration testing before and it makes repeat runs more reliable, ie by cleaning up before the tests run, if you cancel prior to tests finishing you are still able to re-run without worrying about old state. Also, if you run into a test failure, now you have data post-test run that you can dig into and investigate without modifying your test infrastructure ad-hoc.