Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

METRON-1971: Short timeout value in Cypress may cause build failures #1323

Closed
wants to merge 1 commit into from

Conversation

sardell
Copy link
Contributor

@sardell sardell commented Jan 25, 2019

Contributor Comments

Link to original Jira ticket: https://jira.apache.org/jira/browse/METRON-1971

We currently use the default timeout in Cypress, which is only 4000ms. I believe this short timeout can cause failures like this in Metron: https://travis-ci.org/apache/metron/jobs/483945575

In this Pull Request, I increased Cypress' default timeout and request timeout settings (the request timeout was only 5000ms).

Testing

  • With the current version of master, open a terminal window and navigate to metron/metron-interface/metron-alerts
  • Generate an Angular build by running npm run build.
  • Start a production-configured angular server by running npm run start:ci.
  • Start the Cypress UI by running npm run cypress:open.
  • Start the tests by clicking on the PCAP test suite in the Cypress UI.
  • Open the network tab and throttle network speeds to 'Slow 3G'.
  • Re-run the tests by clicking on the reload page button. Your tests should fail.

To confirm the changes work, clone this repo and run the steps above. The tests should pass despite the slow time to render.

Pull Request Checklist

Thank you for submitting a contribution to Apache Metron.
Please refer to our Development Guidelines for the complete guide to follow for contributions.
Please refer also to our Build Verification Guidelines for complete smoke testing guides.

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.

@justinleet
Copy link
Contributor

Typically for these sorts of intermittent failures, we tend to do a few runs to try to make sure the issue is taken care (e.g. close and reopen the PR a set of times to force Travis reruns). Have you done something similar while testing this change?

@ottobackwards
Copy link
Contributor

Anything based on a timeout, when executed in travis, in the Apache queue is going to be unreliable at some point isn't it?

@sardell sardell closed this Jan 28, 2019
@sardell sardell reopened this Jan 28, 2019
@sardell sardell closed this Jan 28, 2019
@sardell sardell reopened this Jan 28, 2019
@sardell
Copy link
Contributor Author

sardell commented Jan 28, 2019

@ottobackwards I agree, anything based on a time will eventually be unreliable at some point. In this case, I am overriding a value that acts as a safety because Cypress will re-run assertions over and over until they are true or reach the timeout value specified. Indeed, it would be better if there was a way for Cypress to determine when to exit an assertion that wasn't time-based, but I'm guessing they chose to do things this way because of the nature of e2e testing in a browser. For example, in the case where the UI builds fine but contains an error that prevents part of the page from rendering, Cypress will re-run an assertion on a non-existing element over and over. Without a timeout value, it will do so infinitely. Protractor has similar timeout configurations: https://github.com/angular/protractor/blob/master/docs/timeouts.md

@sardell sardell closed this Jan 28, 2019
@sardell sardell reopened this Jan 28, 2019
@tiborm
Copy link
Contributor

tiborm commented Jan 28, 2019

@ottobackwards for me it seems like timers and timeouts affected by the load of the Travis executor.

@ottobackwards
Copy link
Contributor

Right, we have seen issues with things in the apache queue before. I guess we can just do our best and handle it as it goes.

@sardell
Copy link
Contributor Author

sardell commented Jan 28, 2019

@justinleet Thanks for cluing me in on standard practice with these sorts of issues. I closed and reopened a handful of times and the tests continually pass. Hopefully that gives us some sort of peace of mind. I guess that's the fun of dealing with intermittent failures in Travis.

@justinleet
Copy link
Contributor

I'm +1 for this. Thanks!

@ottobackwards Do you have any concerns beyond the earlier discussion that we want to address right now?

@ottobackwards
Copy link
Contributor

I don't think that tests that can be variable, like we can never be sure if they are failing or working because of the test env should be tied to the main travis and fail the whole build. This leads to not trusting travis. I don't have a way to resolve that however.

+0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants