[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user iraghumitra commented on the issue: https://github.com/apache/metron/pull/857 Got busy with too many things happening around me, will yield this a conclusion next week. ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/857 What is the status of this pr? it is 29 day without comment, and conflicted, literally, and perhaps figuratively ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user merrimanr commented on the issue: https://github.com/apache/metron/pull/857 @justinleet I believe @iraghumitra is still working on the SELENIUM_PROMISE_MANAGER change proposed above but I will defer to him. ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/857 @merrimanr @mmiklavc @iraghumitra What are the next steps towards being able to have this stable? It sounds like there was some playing around with versions, and that there may be some more structural changes to the code style. How do you propose we move forward? I'd really like to see this stuff made stable and I'm happy to help out with more testing or whatever else we need. ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/857 Follow up from @merrimanr and my work yesterday. We upped the versions of Node to 9.2.1. Per the doc, >8 is required to work with async/await. For good measure, I also set the NPM version to 5.6.0. We didn't touch Jasmine, but the Protractor docs also state that it should be > 2.7. Looks like we are currently using 2.5.2 per the package.json file. We may want to consider increasing that version as well. We added `SELENIUM_PROMISE_MANAGER: false` to `protractor.conf.js` and immediately got failures due to `Promise` use in the Protractor tests and configuration. e.g. `var defer = protractor.promise.defer();`. So we removed references to promises in the conf file and were able to get past that first batch of errors. Now we were into problems with the tests. I started with the `login.e2e-spec.ts` spec file and removed `: Promise`. Running the tests again, the login tests were able to succeed. There are still a large number of failures due to disabling the promise manager, but still having code throughout the test suite that leverages the older style. It's unclear if this will resolve all stability issues, but I think this is moving in the right direction. ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/857 OK, I was able to get the tests to run. First run I get a single error. @merrimanr and I are talking through it now. ``` [INFO] Test spec for login page [INFO] â should display error message for invalid credentials [INFO] - Failed: Error while waiting for Protractor to sync with the page: true [INFO] .â should login for valid credentials [INFO] .â should logout ... [INFO] Failures: [INFO] 1) Test spec for login page should display error message for invalid credentials [INFO] Message: [INFO] Failed: Error while waiting for Protractor to sync with the page: true [INFO] Stack: [INFO] Error: Error while waiting for Protractor to sync with the page: true [INFO] at runWaitForAngularScript.then (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/built/browser.js:463:23) [INFO] at ManagedPromise.invokeCallback_ (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:1376:14) [INFO] at TaskQueue.execute_ (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:3084:14) [INFO] at TaskQueue.executeNext_ (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:3067:27) [INFO] at asyncRun (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:2927:27) [INFO] at /Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:668:7 [INFO] at process._tickCallback (internal/process/next_tick.js:109:7) [INFO] From: Task: [INFO] at pollCondition (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:2195:19) [INFO] at /Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:2191:7 [INFO] at new ManagedPromise (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:1077:7) [INFO] at ControlFlow.promise (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:2505:12) [INFO] at /Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:2190:22 [INFO] at TaskQueue.execute_ (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:3084:14) [INFO] at TaskQueue.executeNext_ (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:3067:27) [INFO] at asyncRun (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:2974:25) [INFO] at /Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:668:7 [INFO] at process._tickCallback (internal/process/next_tick.js:109:7) [INFO] From: Task: [INFO] at scheduleWait (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:2188:20) [INFO] at ControlFlow.wait (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/promise.js:2517:12) [INFO] at Driver.wait (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/node_modules/selenium-webdriver/lib/webdriver.js:934:29) [INFO] at run (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/built/browser.js:59:33) [INFO] at ProtractorBrowser.to.(anonymous function) [as wait] (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/node_modules/protractor/built/browser.js:67:16) [INFO] at Object.waitForElementVisibility (/Users/mmiklavcic/devprojects/metron/metron-interface/metron-alerts/e2e/utils/e2e_util.ts:43:20) [INFO] at LoginPage.setUserNameAndPassword
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/857 Should we also run the tests using the new Maven profile? `mvn test -Pe2e` The README should probably be updated to reflect this as well. ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/857 A couple notes for the community - if your node/npm versions are out of date and you've already built the project, you'll run into trouble with versions not matching against the expected node version, e.g. `Node Sass could not find a binding for your current environment: OS X 64-bit with Node.js 9.x`. I deleted the node_modules directory entirely, `rm -r metron-alerts/node_modules`, re-ran the install, `npm install`, and then the start server script ran successfully. Just to note, the server script does not background the process, so you'll need to manually do that or use 2 windows to run the e2e tests. ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/857 Seems that my NodeJS and npm versions got mangled since last time I ran this. Perhaps migrating laptops, upgrading to High Sierra, and switching from MacPorts to HomeBrew might have something to do with it ;) ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/857 I've hit the same intermittent errors trying this, e.g. ``` [INFO] - Failed: unknown error: Element ... is not clickable at point (1279, 95). Other element would receive the click: ... ``` One of the very first things that failed was ``` [INFO] â should display error message for invalid credentials [INFO] - Failed: Error while waiting for Protractor to sync with the page: true ``` Is it possible there's an intermittent error getting the tests initially lined up that just carries through everything? @iraghumitra Can you also update the README with the instructions for running this with the mvn profile + any troubleshooting or other steps that need to happen? ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user iraghumitra commented on the issue: https://github.com/apache/metron/pull/857 @merrimanr renamed the variable POLLING_DEFAULT_STATE to DISABLE_POLLING ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user iraghumitra commented on the issue: https://github.com/apache/metron/pull/857 @merrimanr - I removed the node version check in the ui code - The browser.sleep is used in following areas - When we are using methods like waitForText, few times I see the test fails saying that that it got the text we were waiting to change which doesn't make sense. So, in this case, i am just yielding the main thread with a sleep of 500 milliseconds. - Create/Add meta-alert, these calls takes few seconds at times. - When we use animation to slide in the details pane, we don't have a class that gets added/removed for this so it makes it difficult for us to know when the pane is loaded. We can add/remove a class for animation by moving the slider to a component or directive. This might be out of scope for current PR. - Refactored tests for Facets - Looks like the meta alert creation failed can you attach the stack trace it might give me additional info to debug. ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user merrimanr commented on the issue: https://github.com/apache/metron/pull/857 Took a first pass at this and I feel like the e2e tests are much improved. Great progress and good job so far. I was able to get several successful runs whereas before it was difficult to get a single successful run. A couple of comments/suggestions: - I noticed a warning message stating I wasn't on a recent enough version of nodejs. I think we should be using the maven frontend plugin to run tests so that we guarantee a consistent version is used. I submitted a [PR](https://github.com/iraghumitra/incubator-metron/pull/6) as an example of how to do this. - I noticed there are still several browser.sleep statements throughout the tests (I counted 20). I think our goal should be to remove all of them. I know some of these should definitely be removed (alerts-list.po.ts, line 87) and may have just been missed. If there are cases where we MUST have them, I think those cases needed to be discussed and justified. - I feel like the "should expand all facets" and "should collapse all facets" tests in alert-filters.e2e-spec.ts are unnecessary (and would allow us to remove a couple browser.sleep statements). These tests are simply opening and closing bootstrap widgets which is controlled by code we don't maintain (bootstrap). I would instead prefer a test that selects a filter and checks that the search box and results are properly updated. - I have ran into these errors a couple times. Not sure if I just ended up in a bad state somehow or if it's because I'm running tests through Maven: ``` â should have all the steps for meta alerts workflow - Failed: unknown error: Element is not clickable at point (1278, 102). Other element would receive the click: ... â should create a meta alert from nesting of more than one level - Failed: unknown error: Element is not clickable at point (1350, 503). Other element would receive the click: ... ``` ---
[GitHub] metron issue #857: METRON-1340: Improve e2e tests for metron alerts
Github user justinleet commented on the issue: https://github.com/apache/metron/pull/857 I haven't looked at the code yet, but I was able to pull this down and run the e2e tests repeatedly without failures, so this is definitely great stuff. ---
