{"action":"created","issue":{"url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/325","labels_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/325/labels{/name}","comments_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/325/comments","events_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/325/events","html_url":"https://github.com/thaliproject/Thali_CordovaPlugin/pull/325","id":116785108,"number":325,"title":"Enable UnitTest","user":{"login":"tobybrad","id":8226225,"avatar_url":"https://avatars.githubusercontent.com/u/8226225?v=3","gravatar_id":"","url":"https://api.github.com/users/tobybrad","html_url":"https://github.com/tobybrad","followers_url":"https://api.github.com/users/tobybrad/followers","following_url":"https://api.github.com/users/tobybrad/following{/other_user}","gists_url":"https://api.github.com/users/tobybrad/gists{/gist_id}","starred_url":"https://api.github.com/users/tobybrad/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tobybrad/subscriptions","organizations_url":"https://api.github.com/users/tobybrad/orgs","repos_url":"https://api.github.com/users/tobybrad/repos","events_url":"https://api.github.com/users/tobybrad/events{/privacy}","received_events_url":"https://api.github.com/users/tobybrad/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/labels/2%20-%20Ready","name":"2 - Ready","color":"CCCCCC"},{"url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/labels/in%20progress","name":"in progress","color":"ededed"}],"state":"open","locked":false,"assignee":{"login":"tobybrad","id":8226225,"avatar_url":"https://avatars.githubusercontent.com/u/8226225?v=3","gravatar_id":"","url":"https://api.github.com/users/tobybrad","html_url":"https://github.com/tobybrad","followers_url":"https://api.github.com/users/tobybrad/followers","following_url":"https://api.github.com/users/tobybrad/following{/other_user}","gists_url":"https://api.github.com/users/tobybrad/gists{/gist_id}","starred_url":"https://api.github.com/users/tobybrad/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tobybrad/subscriptions","organizations_url":"https://api.github.com/users/tobybrad/orgs","repos_url":"https://api.github.com/users/tobybrad/repos","events_url":"https://api.github.com/users/tobybrad/events{/privacy}","received_events_url":"https://api.github.com/users/tobybrad/received_events","type":"User","site_admin":false},"milestone":null,"comments":96,"created_at":"2015-11-13T15:04:25Z","updated_at":"2015-12-08T07:22:07Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/pulls/325","html_url":"https://github.com/thaliproject/Thali_CordovaPlugin/pull/325","diff_url":"https://github.com/thaliproject/Thali_CordovaPlugin/pull/325.diff","patch_url":"https://github.com/thaliproject/Thali_CordovaPlugin/pull/325.patch"},"body":"\n\n<!-- Reviewable:start -->\n[<img src=\"https://reviewable.io/review_button.png\" height=40 alt=\"Review on Reviewable\"/>](https://reviewable.io/reviews/thaliproject/thali_cordovaplugin/325)\n<!-- Reviewable:end -->\n\r\n\r\n<!---\r\n@huboard:{\"order\":0.0048828125,\"milestone_order\":325,\"custom_state\":\"\"}\n-->\r\n"},"comment":{"url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/comments/162798965","html_url":"https://github.com/thaliproject/Thali_CordovaPlugin/pull/325#issuecomment-162798965","issue_url":"https://api.github.com/repos/thaliproject/Thali_CordovaPlugin/issues/325","id":162798965,"user":{"login":"vjrantal","id":207474,"avatar_url":"https://avatars.githubusercontent.com/u/207474?v=3","gravatar_id":"","url":"https://api.github.com/users/vjrantal","html_url":"https://github.com/vjrantal","followers_url":"https://api.github.com/users/vjrantal/followers","following_url":"https://api.github.com/users/vjrantal/following{/other_user}","gists_url":"https://api.github.com/users/vjrantal/gists{/gist_id}","starred_url":"https://api.github.com/users/vjrantal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/vjrantal/subscriptions","organizations_url":"https://api.github.com/users/vjrantal/orgs","repos_url":"https://api.github.com/users/vjrantal/repos","events_url":"https://api.github.com/users/vjrantal/events{/privacy}","received_events_url":"https://api.github.com/users/vjrantal/received_events","type":"User","site_admin":false},"created_at":"2015-12-08T07:22:07Z","updated_at":"2015-12-08T07:22:07Z","body":"\n\n\n\nReview status: 15 of 37 files reviewed at latest revision, 9 unresolved discussions.\n\n---\n\n*[test/TestServer/Config_PerfTest.json, line 5 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/thali_cordovaplugin/325#-K4hQfSBjI0UNbh7IJj6:-K5-LedsVWyx4b7C1IlH:1086138475) ([raw file](https://github.com/thaliproject/thali_cordovaplugin/blob/93b632f224a3be3a1809322bbdbcf110a28860da/test/TestServer/Config_PerfTest.json#L5)):*\nAgreed.\n\nCould we do so that we would leave this code as is in the PR and open up an issue about making the `round` behave so that it can be used to run the specified test multiple times?\n\n---\n\n*[test/TestServer/index.js, line 32 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/thali_cordovaplugin/325#-K4hWhPrHIKyrlDtQ79_:-K5-LzaxBaU4el5ysECe:-2021969822) ([raw file](https://github.com/thaliproject/thali_cordovaplugin/blob/c5de1fe0b94e141bea7f96af0abb6231e9e52dd8/test/TestServer/index.js#L32)):*\nIt is true that it is to work around issues. Mainly, the issue we still see is that when deploying to 20+ Android devices, sometimes something might go wrong (typically, we tend to say `adb` has issues). If an alternative solution is found for the root cause, the code here could be gotten rid of.\n\n---\n\n*[test/TestServer/PerfTestFramework.js, line 128 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/thali_cordovaplugin/325#-K4vAdHwl8XsPo15YMi6:-K5-N3wVtMZlpGSc4Mk3:-296332589) ([raw file](https://github.com/thaliproject/thali_cordovaplugin/blob/c5de1fe0b94e141bea7f96af0abb6231e9e52dd8/test/TestServer/PerfTestFramework.js#L128)):*\nIn the CI, what often happens is that not all devices manage to run the entire test in the time given for the CI run. In that case, the test server signals devices that the time is up and devices send the report from the results they have gathered so far.\n\nI think this step is important, because otherwise, we wouldn't see the results from devices that don't fully complete even though they might have useful data.\n\n---\n\n*[test/www/jxcore/package.json, line 37 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/thali_cordovaplugin/325#-K4vGvV0-8vONZRwPVIG:-K5-OPfoSBj3MFBvCR0T:-65698632) ([raw file](https://github.com/thaliproject/thali_cordovaplugin/blob/c5de1fe0b94e141bea7f96af0abb6231e9e52dd8/test/www/jxcore/package.json#L37)):*\nThe ^ does not mean same as >, but my point wasn't really about the version.\n\nIt was more about in which scenarios we want to run these tests against the thali package coming from npm (instead of a git clone). If that scenario is wanted to be supported, this change is justified.\n\n---\n\n*[test/www/jxcore/perf_tests/PerfTestFrameworkClient.js, line 153 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/thali_cordovaplugin/325#-K4vI0WKr78DQWx9pJnQ:-K5-PoGylv1Z4wmGRKLB:-1545402182) ([raw file](https://github.com/thaliproject/thali_cordovaplugin/blob/c5de1fe0b94e141bea7f96af0abb6231e9e52dd8/test/www/jxcore/perf_tests/PerfTestFrameworkClient.js#L153)):*\nWas referring to the change serveraddress.json -> server-address.js, but you had fixed this already so all good :)\n\nWith the inline-require, I meant that typically, the requires are at the top of the file, but now that I look at the latest code, I think you have it inline for better mock-up capabilities so I think that is okay.\n\n---\n\n*[test/www/jxcore/perf_tests/PerfTestFrameworkClient.js, line 191 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/thali_cordovaplugin/325#-K4vIbxXyeNQk3pSkFjn:-K5-RavMKO5yQDVg6ffC:-1889537009) ([raw file](https://github.com/thaliproject/thali_cordovaplugin/blob/c5de1fe0b94e141bea7f96af0abb6231e9e52dd8/test/www/jxcore/perf_tests/PerfTestFrameworkClient.js#L191)):*\nThat is good point that I forgot / overlooked. The idea was that if there is a connection failure to the test server, the device still prints the results so that they are accessible \"somewhere\" after the run.\n\nAn idea was that we could reuse the same result printing code in both sides, but this was just never done. I think it is okay to leave this in any way you want within this PR.\n\n---\n\n*[test/www/jxcore/PerfTest_app.js, line 73 \\[r1\\]](https://reviewable.io:443/reviews/thaliproject/thali_cordovaplugin/325#-K4vKfd5hCb3RZMVWj_P:-K5-SQwr4NqkixRUuvPK:1781024794) ([raw file](https://github.com/thaliproject/thali_cordovaplugin/blob/c5de1fe0b94e141bea7f96af0abb6231e9e52dd8/test/www/jxcore/PerfTest_app.js#L73)):*\nWe have a work item to ensure they are really async https://github.com/thaliproject/Thali_CordovaPlugin/issues/365 so it might be good \"future-proofing\" that this code works after that issue is fixed.\n\n---\n\n\n*Comments from the [review on Reviewable.io](https://reviewable.io:443/reviews/thaliproject/thali_cordovaplugin/325)*\n<!-- Sent from Reviewable.io -->\n"}}