[GitHub] szha commented on issue #8689: Mark tests that should only be run nightly.

2017-11-19 Thread GitBox
szha commented on issue #8689: Mark tests that should only be run nightly.
URL: https://github.com/apache/incubator-mxnet/pull/8689#issuecomment-345609106
 
 
   @KellenSunderland please rebase and resolve conflict.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] szha commented on issue #8689: Mark tests that should only be run nightly.

2017-11-17 Thread GitBox
szha commented on issue #8689: Mark tests that should only be run nightly.
URL: https://github.com/apache/incubator-mxnet/pull/8689#issuecomment-345338868
 
 
   @KellenSunderland thanks. The information you collected in this PR while 
analyzing the tests are valuable. It would be great if we could have a summary 
and a checklist of the test problems in an issue, so that everyone can pitch in 
on helping and see the progress.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] szha commented on issue #8689: Mark tests that should only be run nightly.

2017-11-17 Thread GitBox
szha commented on issue #8689: Mark tests that should only be run nightly.
URL: https://github.com/apache/incubator-mxnet/pull/8689#issuecomment-345336009
 
 
   I understand the motivation for moving tests to nightly. The concern I still 
have is if we disable certain tests on PR-level and only run them nightly, the 
problems in code may not surface in PR as they should. One might submit a patch 
for optimization or fix on existing op which actually contains bug, whose tests 
only run nightly. Reviewers might assume that the tests are run by the CI while 
in fact they are not. In general, removing tests from CI in PR is equivalent of 
removing the guarantee that the particular functionality still works after the 
PR.
   
   Admittedly some tests, including some of the tests I wrote, may be simple 
fuzzy tests with randomized inputs, that should have been designed as small 
fixtures to catch specific edge cases. That way, the each of the tests won't 
run for longer than seconds which is ideal. I agree we should do better at 
writing tests. Given the current situation in the test cases, it will be major 
endeavor moving from where we are to where the ideal world is. Given the 
constraint that we shouldn't be breaking mxnet in doing so, I think we should 
only replace existing long-running tests with carefully designed test cases 
instead of disabling them.


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:
us...@infra.apache.org


With regards,
Apache Git Services