[MXNET-644] Automated flaky test detection #11991

cetsai · 2018-08-02T02:21:31Z

Description

This PR adds the necessary components for an automated flaky tests detection measure, the design of which is detailed on the wiki.

These components, diff collator, dependency analyzer, and flakiness checker are used by the check_flakiness script, which will be run in a Jenkins pipeline to automatically check PRs for flaky tests. Once active, the tool will mark PRs that cause flaky tests so that they can be fixed before being merged with master.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
Changes are complete (i.e. I finished coding on this PR)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

Moved flakiness_checker.py to flaky_tests folder, along with other improvements
Added script, check_flakiness.py, which will be used in a Jenkins pipeline to check commits for flaky tests
Added Jenkinsfile and docker run-time function to automate the checking of commits for flaky tests

cetsai · 2018-08-02T02:21:51Z

@marcoabreu @haojin2

haojin2 · 2018-08-02T03:32:37Z

Why do you have some updates in mshadow and tvm? Did you do git submodule update --init --recursive before you commit any changes?

apeforest

Some reviews for now

apeforest · 2018-08-03T17:37:27Z

tools/flaky_tests/check_branch.py

+    return [(filename, test) 
+            for filename in deps.keys() 
+            for test in deps[filename] 
+            if test.startswith("test_")]


Make this "test_" a constant header and document this clearly.

apeforest · 2018-08-03T17:39:51Z

tools/flaky_tests/check_branch.py

+    for t in tests:
+        total_time += time_test(t)
+
+    n = int(TIME_BUDGET / total_time)


Need to handle divide-by-zero.

apeforest · 2018-08-03T17:41:06Z

tools/flaky_tests/check_branch.py

+
+
+def output_results(flaky, nonflaky):
+    print("Following tests failed flakiness checker:")


Why not use logger?

Usually my approach is to use logging for outputting information about program execution and using print for actual output. However, I'd be fine with switching over to logging, if that's more in line with what MXNet does.

Using logging for everything is preferred in general

tools/flaky_tests/dependency_analyzer.py

cetsai · 2018-08-06T19:01:27Z

@marcoabreu, could you take a look at least at the Jenkinsfile to see if I'm missing anything

marcoabreu · 2018-08-08T09:06:54Z

tools/flaky_tests/Jenkinsfile

+        node('mxnetlinux-gpu') {
+            ws('workspace/flakiness_check'){
+                init_git()
+                docker_run('ubuntu_gpu', 'run_flakiness_checker', true)


Does it not need a compiled version of MXNet?

right, for now I'll build mxnet each time we run this, but really we should only do so when dependency analyzer detects changed tests. This would take some refactoring, however, and I think it would be good to get a version of this tool running ASAP.

marcoabreu · 2018-08-08T16:29:19Z

Sorry, I'm currently swamped with high priority tasks and I don't have time to review your pull request.

reorganized code changed diff collator output added logging and improved command-line options Removed extra space added some comments fixed documentation create folder

wip on chack_branch changed logging and added support for cross-file dependnecies finished basic check_branch

# The first commit's message is: check_branch is demo-ready # This is the 2nd commit message: renamed test_selector to dependency_analyzer # This is the 3rd commit message: fixed check_branch output # This is the 4th commit message: refactoring # This is the 5th commit message: improved logging

renamed test_selector to dependency_analyzer fixed check_branch output refactoring improved logging wip ci deployment changde logging levels code improvment

changed Jenkinsfile to use redesigned system included config file for dependency analyzer minor fixes

marcoabreu · 2018-08-31T11:32:31Z

tools/flaky_tests/Jenkinsfile

+    // only continue if some tests were selected
+    if( ! tests ) {
+        currentBuild.result = 'SUCCESS'
+        return


Please don't return here. The wrapping logic (utils.main_wrapper) is taking care of propagating the results properly. Just do nothing in that case.

marcoabreu · 2018-08-31T11:32:59Z

tools/flaky_tests/Jenkinsfile

+                utils.init_git()
+                utils.docker_run('ubuntu_cpu', 'select_tests', false)
+                tests = fileExists('tests.tmp')
+                stash name:'flaky_tests', includes:'tests.tmp' 


marcoabreu · 2018-08-31T11:33:12Z

tools/flaky_tests/Jenkinsfile

+        node(NODE_LINUX_CPU){
+            ws('workspace/fc-preprocessing'){
+                utils.init_git()
+                utils.docker_run('ubuntu_cpu', 'select_tests', false)


select_tests does not exist

nice catch, fixed

tools/flaky_tests/Jenkinsfile

cetsai · 2018-09-04T15:14:43Z

Thanks for the reviews @marcoabreu anything else needed in order to merge?

lebeg · 2018-09-06T14:52:05Z

There is an error on the run job: http://jenkins.mxnet-ci-dev.amazon-ml.com/job/flaky-test-detector/view/change-requests/job/PR-11991/4/console

+ NOSE_COVERAGE_ARGUMENTS='--with-coverage --cover-inclusive --cover-xml --cover-branches --cover-package=mxnet'
+ set +x
+ tool/flaky_test_bot/test_selector.py -b HEAD~1 HEAD
/work/runtime_functions.sh: line 1012: tool/flaky_test_bot/test_selector.py: No such file or directory

@cetsai would you be able to look into it?

cetsai · 2018-09-06T14:57:38Z

@lebeg it was fixed in the last commit

E: sorry, I forgot I changed the directory name

lebeg · 2018-09-06T15:21:28Z

Have you tried to test your changes locally? The command would be:

ci/build.py -p ubuntu_cpu /work/runtime_functions.sh flaky_check_select_tests

and

ci/build.py -p ubuntu_gpu /work/runtime_functions.sh flaky_check_run_flakiness_checker

haojin2 · 2018-09-12T17:30:36Z

ci/docker/runtime_functions.sh

+}
+flaky_check_run_flakiness_checker(){
+    set -ex
+    export PYTHONPATH=./python/


Should this line be export PYTHONPATH=/work/mxnet/python/ like here: https://github.com/apache/incubator-mxnet/pull/11991/files#diff-1335fbaf3930b1438d9be18edb07a1a6R921 ?

not sure, I was looking at the python2 unit test runtime function, which uses export PYTHONPATH=./python/ https://github.com/apache/incubator-mxnet/blob/64566872a28a9426f3ec20bcf0210ebb608854f8/ci/docker/runtime_functions.sh#L639

But the pipeline is failing on not able to import mxnet here: http://jenkins.mxnet-ci-dev.amazon-ml.com/blue/organizations/jenkins/flaky-test-detector/detail/PR-11991/11/pipeline/67. Maybe we should switch to /work/mxnet/python/?

Perhaps-- however, that run was triggered before the last commit, so I don't know which of these options we should go with. Perhaps @marcoabreu can help?

@marcoabreu could you give some help here to get this to work?

@haojin2 @cetsai @lebeg @marcoabreu requesting an update on this

How can I help?

@haojin2 I'm having trouble running this locally, can you try ?

@haojin2 @cetsai Were you able to run this locally?

Sorry, don't have any cycles for this at this moment...

roywei · 2018-10-29T22:18:19Z

ping @cetsai any updates?

anirudhacharya · 2018-11-13T00:34:05Z

@nswamy @sandeep-krishnamurthy can you please close this PR.

@cetsai feel free to reopen the PR once the changes are ready.

stu1130 · 2018-11-20T22:25:31Z

@nswamy @sandeep-krishnamurthy can you please close this PR.

cetsai requested a review from szha as a code owner August 2, 2018 02:21

cetsai force-pushed the flaky_test_bot branch 4 times, most recently from 9b3edb9 to 8ec8f08 Compare August 2, 2018 17:06

apeforest suggested changes Aug 3, 2018

View reviewed changes

marcoabreu reviewed Aug 8, 2018

View reviewed changes

vandanavk mentioned this pull request Aug 8, 2018

47 undefined variable errors with Pylint #11904

Closed

cetsai and others added 10 commits August 8, 2018 15:36

Add diff collator

e3b8150

reorganized code changed diff collator output added logging and improved command-line options Removed extra space added some comments fixed documentation create folder

added check_branch script

ea6a716

added test_selector

d7a5f38

wip on chack_branch changed logging and added support for cross-file dependnecies finished basic check_branch

check_branch is demo-ready

b02ac3d

renamed test_selector to dependency_analyzer fixed check_branch output refactoring improved logging wip ci deployment changde logging levels code improvment

ready for deployment

57859b2

renamed directory, removed old flakiness_checker

9d91acf

added lisence to dependency_analyzer.py

3a703c2

update submodule

b40e6ab

Implemented requested changes

f9ec677

cetsai force-pushed the flaky_test_bot branch from f77acff to b3c1e75 Compare August 8, 2018 22:37

changed print statments to use logging

46b3c0f

changed Jenkinsfile to use redesigned system included config file for dependency analyzer minor fixes

cetsai force-pushed the flaky_test_bot branch from b3c1e75 to 46b3c0f Compare August 8, 2018 22:40

nswamy added Feature pr-awaiting-review PR is waiting for code review Build labels Aug 9, 2018

szha removed their request for review August 9, 2018 19:12

cetsai force-pushed the flaky_test_bot branch from c51aab2 to 67d5c27 Compare August 15, 2018 23:29

marcoabreu reviewed Aug 31, 2018

View reviewed changes

fixed Jenkinsfile formatting and runtime function calls

2c1d669

cetsai force-pushed the flaky_test_bot branch from 45fa6d4 to 2c1d669 Compare August 31, 2018 18:12

marcoabreu reviewed Sep 2, 2018

View reviewed changes

tools/flaky_tests/Jenkinsfile Outdated Show resolved Hide resolved

tools/flaky_tests/Jenkinsfile Outdated Show resolved Hide resolved

added mxnetlinux-cpu to node labels, changed compilation to cpu stage

fa27648

cetsai force-pushed the flaky_test_bot branch from acb5642 to fa27648 Compare September 4, 2018 07:06

lebeg approved these changes Sep 4, 2018

View reviewed changes

cetsai force-pushed the flaky_test_bot branch 3 times, most recently from 72b63e3 to a442e7a Compare September 6, 2018 20:04

fix error in runtime functions

300492b

cetsai force-pushed the flaky_test_bot branch from a442e7a to 300492b Compare September 6, 2018 23:03

Carl Tsai added 5 commits September 7, 2018 03:43

changed permissions

146af59

use utf-8 encoding

86dee1a

fix variable names

d7f679c

fix Jenkinsfile

1265b91

export python path in runtime functions

9054e72

haojin2 reviewed Sep 12, 2018

View reviewed changes

szha added Feature request and removed Feature labels Nov 14, 2018

sandeep-krishnamurthy closed this Nov 27, 2018



		def output_results(flaky, nonflaky):
		print("Following tests failed flakiness checker:")

[MXNET-644] Automated flaky test detection #11991

[MXNET-644] Automated flaky test detection #11991

Conversation

cetsai commented Aug 2, 2018 • edited Loading

Description

Checklist

Essentials

Changes

cetsai commented Aug 2, 2018

haojin2 commented Aug 2, 2018

apeforest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cetsai Aug 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cetsai commented Aug 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcoabreu commented Aug 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cetsai commented Sep 4, 2018

lebeg commented Sep 6, 2018

cetsai commented Sep 6, 2018 • edited Loading

lebeg commented Sep 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roywei commented Oct 29, 2018

anirudhacharya commented Nov 13, 2018

stu1130 commented Nov 20, 2018

cetsai commented Aug 2, 2018 •

edited

Loading

cetsai Aug 3, 2018 •

edited

Loading

cetsai commented Sep 6, 2018 •

edited

Loading