platforms.py hardcodes a mapping between builder names for test jobs and builder names for build jobs. Ideally, buildbot would provide this information (see bug 1129594) but, since what needs to be changed for that to happen isn't clear at the moment, a good first step is to make the mapping better and test it thoroughly.

This with regards to https://github.com/armenzg/mozilla_ci_tools/blob/master/mozci/platforms.py This will help mozilla_ci_tools not to fall out of date, hence, helping bug 1120998. Thanks adusca!

Looking at allthethings.json I believe that associated_build_job is determined by the properties "branch", "platforms" and "product", e.g. everything that has "properties": { "branch": "ash", "platform": "macosx64", "product": "firefox", should return OS X 10.7 ash build and luckily OS X 10.7 ash build also has the same combo. So all I needed to do was figure out which builders were build and which were tests, and I did this by looking at property 'slavebuilddir'. A disclaimer: My unit tests don't pass yet. Also, I look at "stage_platform" if it exists in order to catch PGO builds, but in some cases it would be better to look at the "platform" information (for example, OS X 10.10 has platform "yosemite" and stage_platform "macosx64"). This also needs to be investigated in the context of Windows versions.

Hi Alice, Thank you for your patches. In order to make them easy for me to review would you mind using "git diff > filename.diff" and attach that? Remember to also mark the checkbox "patch". That allows bugzilla to add review visualizations. For instance, to see your changes, I have to do this: > wget -q https://bugzilla.mozilla.org/attachment.cgi?id=8560220 > mv attachment.cgi\?id\=8560220 mozci/platforms.py > git diff At that point I can see clearly what you change.

I will get back to you soon about the reviews.

Comment on attachment 8560464 [details] [diff] [review] diff of attachment 8560220 [details] Review of attachment 8560464 [details] [diff] [review]: ----------------------------------------------------------------- Once you address my comments I can test it and land it for you. ::: mozci/platforms.py @@ -7,2 @@ > > -LOG = logging.getLogger() We need to keep all of this. Use `git pull` to fetch the latest code from the "master" branch. @@ +10,5 @@ > + # skipping these for now > + if 'l10n' in build or 'nightly' in build: > + continue > + props = data['builders'][build]['properties'] > + # we will later associate test job names to the name of the build job Please capitalize the first letter of the beggining of sentences. @@ +13,5 @@ > + props = data['builders'][build]['properties'] > + # we will later associate test job names to the name of the build job > + # that created them. in order to do this, we heuristically figure out > + # what jobs are build jobs by checking the "slavebuilddir" property > + if 'slavebuilddir' not in props or props['slavebuilddir'] != 'test': Very good :) @@ +16,5 @@ > + # what jobs are build jobs by checking the "slavebuilddir" property > + if 'slavebuilddir' not in props or props['slavebuilddir'] != 'test': > + platform = props['platform'] > + if 'stage_platform' in props: > + platform = props['stage_platform'] Add a note on why you do this differently (I believe this applies to PGO builds IIUC). @@ +18,5 @@ > + platform = props['platform'] > + if 'stage_platform' in props: > + platform = props['stage_platform'] > + repo_name = props['branch'] > + BuildJobName[(repo_name, platform, props['product'])] = build When you have a chance, would you mind dumping this data as a file attachment? I would like to see what it looks like :D (/me thinks this is promising) @@ +23,2 @@ > > def associated_build_job(buildername, repo_name): We need to keep a function doc. @@ +25,5 @@ > + props = allthethings.query_job_info(buildername)['properties'] > + platform = props['platform'] > + if 'stage_platform' in props: > + platform = props['stage_platform'] > + return BuildJobName[(props['branch'], platform, props['product'])] This is very sweet :)

In order to generate this patch I did this: git add tests/test_platforms.py git diff --cached > ~/moz/patches/test.diff Every time to make changes you have to `git add` again. You can see the status of files by doing `git status`. I would create a class for each product (Firefox, Mobile, B2G desktop, B2G emulator) and combination of jobs (Opt, Talos, Debug). We can create empty classes for jobs like L10n and Nightly until we implement them. I would also target each job for these branches: mozilla-central, mozilla-inbound, try, mozilla-release, mozilla-esr31 and one of the b2g release branches. I'm so excited to see what you're accomplishing here! I managed to run the tests like this: MacAir ci_tools git:[bisection*] $ nosetests E.....E... ====================================================================== ERROR: generic_test (test_platforms.TestAssociatedBuild) ---------------------------------------------------------------------- TypeError: generic_test() takes exactly 4 arguments (1 given) ====================================================================== ERROR: testUbuntuB2gRepo (test_platforms.TestAssociatedBuild) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/armenzg/repos/ci_tools/tests/test_platforms.py", line 25, in testUbuntuB2gRepo self.generic_test('Ubuntu VM 12.04 mozilla-b2g32_v2_0 debug test marionette', 'mozilla-b2g32_v2_0','Linux mozilla-b2g32_v2_0 leak test build') File "/Users/armenzg/repos/ci_tools/tests/test_platforms.py", line 7, in generic_test self.assertEquals(associated_build_job(buildname,repo_name), expected) File "/Users/armenzg/repos/ci_tools/mozci/platforms.py", line 72, in associated_build_job postfix = JOB_TYPE[b2g_platform][job_type] KeyError: 'Linux' ---------------------------------------------------------------------- Ran 10 tests in 0.012s FAILED (errors=2)

Two of my tests don't pass with this version: 1) The Ubuntu PGO test doesn't work because the test job has a stage_platform = "linux-pgo", but Linux PGO build jobs have platform = "linux". 2) I get "AssertionError: u'Linux mozilla-b2g32_v2_0 build' != 'Linux mozilla-b2g32_v2_0 leak test build'" because the test I checked has platform = "linux", which matches a build job (there is another build job with platform = "linux-debug"). Treeherder infers a "-debug" suffix for the test, which may or may not be correct. I used the Treeherder value as the expected value for the test, though. I still don't have a plan on how do deal with that, but I'm working on improving unittests now.

The tests results I mentioned before: alice@alice-UX303LN:~/mozilla_ci_tools/mozci$ python test_platforms.py .....FE.. ====================================================================== ERROR: testUbuntuPgo (__main__.TestAssociatedBuild) ---------------------------------------------------------------------- Traceback (most recent call last): File "test_platforms.py", line 28, in testUbuntuPgo self.generic_test('Ubuntu HW 12.04 mozilla-central pgo talos svgr-e10s', 'mozilla-central','Linux mozilla-central pgo-build') File "test_platforms.py", line 7, in generic_test self.assertEquals(new_platforms.associated_build_job(buildname,repo_name), expected) File "/home/alice/mozilla_ci_tools/mozci/new_platforms.py", line 35, in associated_build_job associated_build = BuildJobName[(props['branch'], platform, props['product'])] KeyError: (u'mozilla-central', u'linux-pgo', u'firefox') ====================================================================== FAIL: testUbuntuB2gRepo (__main__.TestAssociatedBuild) ---------------------------------------------------------------------- Traceback (most recent call last): File "test_platforms.py", line 25, in testUbuntuB2gRepo self.generic_test('Ubuntu VM 12.04 mozilla-b2g32_v2_0 debug test marionette', 'mozilla-b2g32_v2_0','Linux mozilla-b2g32_v2_0 leak test build') File "test_platforms.py", line 7, in generic_test self.assertEquals(new_platforms.associated_build_job(buildname,repo_name), expected) AssertionError: u'Linux mozilla-b2g32_v2_0 build' != 'Linux mozilla-b2g32_v2_0 leak test build' ---------------------------------------------------------------------- Ran 9 tests in 1.861s FAILED (failures=1, errors=1)

Let's see. For "Ubuntu HW 12.04 mozilla-central pgo talos svgr-e10s" we expect "Linux mozilla-central pgo-build". Your generic call seems to be expressed correctly. For Ubuntu HW 12.04 mozilla-central pgo talos svgr-e10s we should get: [1] "platform": "ubuntu32_hw", "stage_platform": "linux-pgo" "product": "firefox", platform at the end of your run by reading your code should "linux-pgo". So you search for "mozilla-central", "linux-pgo", "firefox" (as we see in the KeyError). Why is Linux mozilla-central pgo-build missing from attachment 8560568 [details]? I think it is because its place has been taken by: > ('mozilla-central', 'linux', 'firefox'): 'Linux mozilla-central build', [2] since you do this: > BuildJobName[(repo_name, props['platform'], props['product'])] = build You should probably check that the key is not there first and raise an error if we're trying to insert on an existing key. It really sucks that the properties between those two jobs are so similar. You probably should append "-pgo" to platform if you find "pgo" in "slavebuilddir". "slavebuilddir": "m-cen-lx-pgo-00000000000000000", It might also be interested to output that information as a sorted list. _I think_ you can do find searches on a list with tuples. Not sure if this is even a good suggestion performance wise. Maybe it doesn't matter since we're not talking about millions of records. On another note, I think we could create a script, that would take all builders and analyze them, only keep the jobs that are triggered by a build and create a data file with information about each test job and its expectation. This could become the fixed data from which your test cases would feed in. This way we test all test jobs against our current mozci. If mozci changes in a wrong way, we will know because the test would fail. If in the future, we want to generate a new file with an updated set of test jobs and expectations; we would have such script to regenerate a new fixed set. I don't know if this makes sense. I haven't thought much about it but it might help test all known cases at this point in time rather than having to enter few test jobs. In other words, I'm saying to write a script to generate the data you were putting in by hand: data = {"FirefoxOpt": [ ('Ubuntu VM 12.04 mozilla-central opt test cppunit', 'Linux mozilla-central build'), ('Ubuntu VM 12.04 x64 mozilla-central opt test mochitest-1', 'Linux x86-64 mozilla-central build'), ('Rev5 MacOSX Mountain Lion 10.8 mozilla-central opt test mochitest-1', 'OS X 10.7 mozilla-central build'), ('Windows 8 64-bit mozilla-central opt test mochitest-1', 'WINNT 6.1 x86-64 mozilla-central build') ], [1] "Ubuntu HW 12.04 mozilla-central pgo talos svgr-e10s": { "properties": { "branch": "mozilla-central", "builddir": "mozilla-central_ubuntu32_hw_test-svgr-e10s", "platform": "ubuntu32_hw", "product": "firefox", "repo_path": "mozilla-central", "script_repo_revision": "production", "slavebuilddir": "test", "stage_platform": "linux-pgo" }, "shortname": "mozilla-central_ubuntu32_hw_test-svgr-e10s-pgo", "slavebuilddir": "test-pgo", "slavepool": "d9e951f02ea8ad5c0684376a23ddf73f212a4d6a" }, [2] "Linux mozilla-central pgo-build": { "properties": { "branch": "mozilla-central", "platform": "linux", "product": "firefox", "repo_path": "mozilla-central", "script_repo_revision": "production" }, "shortname": "mozilla-central-linux-pgo", "slavebuilddir": "m-cen-lx-pgo-00000000000000000", "slavepool": "2e490369b232e800931337b9b1186f5027eac1e6" }, [3] "Linux mozilla-central build": { "properties": { "branch": "mozilla-central", "platform": "linux", "product": "firefox", "repo_path": "mozilla-central", "script_repo_revision": "production" }, "shortname": "mozilla-central-linux", "slavebuilddir": "m-cen-lx-000000000000000000000", "slavepool": "2e490369b232e800931337b9b1186f5027eac1e6" }, https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=e6174a0e5130&exclusion_state=all https://treeherder.mozilla.org/#/jobs?repo=mozilla-b2g32_v2_0&exclusion_state=all&revision=b961877cacba

This is a mapping created from allthethings.json version generated by [1] that I will use to generate tests. [1] https://bug1129594.bugzilla.mozilla.org/attachment.cgi?id=8562079

BuidJobs.json is a mapping from builders to associated build jobs . We can use that to test the function associated_build_jobs and see if we found a good heuristic. Current results are not very promising, but at least now we know what's going on. Tests results with most current version of platforms.py: http://pastebin.mozilla.org/8660718 Tests results with previous version (before patches): http://pastebin.mozilla.org/8660832

Comment on attachment 8562259 [details] [diff] [review] test_platforms.diff Review of attachment 8562259 [details] [diff] [review]: ----------------------------------------------------------------- Overall this is looking great. My apologies for the delayed feedback! I hope you also have other bugs beyond this one! (so you won't get stucked waiting on me) We will need to be able to run this with nose, hence, needing to change this to look like a python unittest file. Can you please move this file under top_folder/tests? (You will to make the directory). On your next patch could you also add the json file so I can run it locally? Also very important, we would probably need to determine what we want to do with calling associated_build_job when we call it for a "build" builder. I think we could either return itself or None. What do you think? ::: mozci/test_platforms.py @@ +11,5 @@ > +import allthethings > +import platforms > + > +# Loading json > +with open('BuildJobs.json', 'r') as f: Could you please call it test_platforms.json? Over time we will have more "test_" files under the tests/ directory and I would like it to be clear which test uses the .json file. @@ +12,5 @@ > +import platforms > + > +# Loading json > +with open('BuildJobs.json', 'r') as f: > + BuildJobs = json.load(f) Could you please call this variable "reference_builders_info"? @@ +14,5 @@ > +# Loading json > +with open('BuildJobs.json', 'r') as f: > + BuildJobs = json.load(f) > + > +data = allthethings._fetch_json() You can append ["builders"] and call the variable builders_data (to make it more clear). @@ +19,5 @@ > + > +ok = 0 > +total = 0 > + > +builders = allthethings.list_builders()[::100] Why are you doing a subset? @@ +21,5 @@ > +total = 0 > + > +builders = allthethings.list_builders()[::100] > +for builder in builders: > + total += 1 I think you can calculate the total value outside of the for loop by using len() on the list of builders.

I updated so now we can run it with: nosetests -i test_platforms.py Here it ran 12576 tests in 2870.404s with errors=1707, failures=5340. I'm assuming that associated_build_jobs of a build job should return itself, but that's easy to change.

This is very very sweet! Thank you adusca! We now have to fix platforms.py to give us the right values! Your json reference file seems good. Could you also fix the issues of running nosetests from one level above? [1] Could you also use `sort` on the json file? [2] I have attached the test run by doing this: nosetests &> ~/moz/patches/errors.txt [1] armenzg@armenzg-thinkpad:~/repos/ci_tools$ nosetests E ====================================================================== ERROR: Failure: IOError ([Errno 2] No such file or directory: 'test_platforms.json') ---------------------------------------------------------------------- Traceback (most recent call last): File "/usr/local/lib/python2.7/dist-packages/nose-1.3.0-py2.7.egg/nose/loader.py", line 286, in generate for test in g(): File "/home/armenzg/repos/ci_tools/tests/test_platforms.py", line 23, in testEverything with open('test_platforms.json', 'r') as f: IOError: [Errno 2] No such file or directory: 'test_platforms.json' ---------------------------------------------------------------------- Ran 1 test in 0.061s FAILED (errors=1) [2] cat test_platforms.json | sort > test_platforms.json2

Comment on attachment 8563489 [details] [diff] [review] test_platforms.diff Review of attachment 8563489 [details] [diff] [review]: ----------------------------------------------------------------- I forgot to mention that we have to adjust this test to use PyUnit: https://docs.python.org/2/library/unittest.html#basic-example ::: tests/test_platforms.py @@ +28,5 @@ > + for build_job in build_jobs: > + reference_builders_info[build_job] = build_job > + > + builders_data = mozci.allthethings._fetch_json()['builders'] > + builders = reference_builders_info.keys() When you switch this to PyUnit you will have to move this block to setUp(). This might be the reason for slow execution.

I'm trying to convert things to PyUnit, but I can't think of another way to get one test per entry in the json file. Any ideas?

I played with this for a while, I see what :adusca has done and it is a good way to go about this. Moving this logic into a setUp routine doesn't work, we end up generating all the info, but at this point nosetests (python unittest) has done all the loading of the test cases, so the dynamic function of creating a test case for each builder type is broken. A few options I see: 1) don't use nosetests 2) hack a bit with unittest.makeSuite() or other functions that are not well documented :) 3) instead of creating a test case for each builder, just do an assert for each builder in a loop

running this in option #3 (http://pastebin.mozilla.org/8719432), yields a key error: ====================================================================== ERROR: test_builders (test_platforms.TestGeneratedBuild) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/jmaher/mozilla/mozilla_ci_tools/tests/test_platforms.py", line 44, in test_builders obtained = mozci.platforms.associated_build_job(builder, repo_name) File "/home/jmaher/mozilla/mozilla_ci_tools/tests/../mozci/platforms.py", line 69, in associated_build_job "%s %s %s" % (PREFIX[prefix], repo_name, JOB_TYPE[job_type]) KeyError: u'Linux x86-64' -------------------- >> begin captured stdout << --------------------- builder: Linux x86-64 mozilla-inbound leak test build repo_name: mozilla-inbound --------------------- >> end captured stdout << ---------------------- ---------------------------------------------------------------------- Ran 1 test in 0.282s this is odd because that is a valid builder, maybe my local environment doesn't have all the data files?

I will think about this on Tuesday (Monday I'm off). For the record, there are needed fixes for platforms.py for the tests to pass.

I generated the previous json assuming that only one build job triggered a given test job and thus wrongly included nightly build jobs in my json file. I'm just fixing that.

This patch depends on https://bug1129594.bugzilla.mozilla.org/attachment.cgi?id=8565192 Since we cannot have builder triggers in allthethings.json, I decided to just use shortnames to try and guess the right trigger name. It's simple and it works pretty well: it passes all 12756 tests! The B2g part needed a one-line hack, though.

Comment on attachment 8565196 [details] [diff] [review] test_platforms.diff After some research I want us to aim to use py.test (pip install pytest). To run the tests we use "py.test". I will look into this a bit.

Comment on attachment 8565206 [details] [diff] [review] platforms.diff This does not apply cleanly. Please use "git pull" before attaching the next patches. Have a look at "git status" to see that all your changes are included. I have landed the braindump change. I hope allthethings.json will update in the next 24 hours.

Comment on attachment 8565196 [details] [diff] [review] test_platforms.diff Review of attachment 8565196 [details] [diff] [review]: ----------------------------------------------------------------- ::: tests/test_platforms.py @@ +9,5 @@ > +import os > +import sys > +import unittest > + > +sys.path.insert(1, os.path.join(sys.path[0], '..')) Please don't use this. You should be able to run "py.test" after you run "python setup.py develop". @@ +11,5 @@ > +import unittest > + > +sys.path.insert(1, os.path.join(sys.path[0], '..')) > + > +import mozci.allthethings It is now "mozci.sources.allthethings". @@ +30,5 @@ > + > +# Build jobs should return themselves > +build_jobs = set(reference_builders_info.values()) > +for build_job in build_jobs: > + reference_builders_info[build_job] = build_job I want associated_build_job() to do this for you.

Hi Alice, your patches are in the right track. The r- is only due to the changes I made last Friday. My apologies for bitrotting your changes!

Hi Alice, I turned the test to follow the py.test framework. It is currently failing due that associated_build_job() does not work for build jobs. Could you please follow this style? It is much leaner than unittest.

I modified the previous test patch to run each test in py.test individually, so now it doesn't stop at the first error. The full output is in [1]. > I want associated_build_job() to do this for you. The new version of platforms.py, which is in the next patch, treats build jobs as well. The for loop in this patch just adds build job test cases, which I decided not to include in the json file itself (all of the test cases in the json file are of the form "test job": "build job", so it's easy to change the desired behavior for build jobs later on if needed). [1] https://bug1130103.bugzilla.mozilla.org/attachment.cgi?id=8565751

This is a rebased version of the platforms.py patch. The remarks in comment 24 still apply.

(Whoops, disregard comment 32 and its attachment: I accidentally had the wrong version of the last patch.) This is a rebased version of the platforms.py patch. The remarks in comment 24 still apply. I've tested it with py.test, and everything works: ============================= test session starts ============================== platform linux2 -- Python 2.7.8 -- py-1.4.23 -- pytest-2.6.0 collected 12756 items test/test_platforms.py ...[dots omitted]... ======================== 12756 passed in 10.91 seconds =========================

Comment on attachment 8565767 [details] [diff] [review] platforms.diff Review of attachment 8565767 [details] [diff] [review]: ----------------------------------------------------------------- Hi Alice! Your code looks great. It simply needs a bit more of comments for people unfamiliar with the data sources. I have landed your code in the "bisecction" branch (I should really rename it to be "experimental" or "trunk"). Could you please deal with the comments by first switching to that branch? (git checkout bisection) Thanks a lot! ::: mozci/platforms.py @@ +14,2 @@ > > +shortname_to_name = {} Could you please add a comment to this variable to help understand what we store in it? @@ +17,2 @@ > > +for build in builders: Could you please add a comment in here as to what we're doing? @@ +25,5 @@ > + # We will later associate test job names to the name of the build job > + # that created them. In order to do this, we heuristically figure out > + # what jobs are build jobs by checking the "slavebuilddir" property > + if 'slavebuilddir' not in props or props['slavebuilddir'] != 'test': > + shortname_to_name[builder['shortname']] = build Is this associating a build to itself? (So we return itself when querying a build job buildername). @@ +27,5 @@ > + # what jobs are build jobs by checking the "slavebuilddir" property > + if 'slavebuilddir' not in props or props['slavebuilddir'] != 'test': > + shortname_to_name[builder['shortname']] = build > + > +for sched, values in data['schedulers'].iteritems(): Could you please add a comment on what we're doing in this block? @@ +31,5 @@ > +for sched, values in data['schedulers'].iteritems(): > + if not sched.startswith('tests-'): > + continue > + > + for builder in values['downstream']: Could you please add a bit of context to this comment? For instance show 2 lines of expected values under "downstream"? Not everyone looking at this code will know what the data looks like. @@ +39,5 @@ > +def associated_build_job(buildername, repo_name): > + '''Given a builder name, find the build job that triggered it. When > + buildername corresponds to a test job, it does so by looking at > + allthethings.json. When buildername corresponds to a build job, it > + passes through unchanged. "it returns it unchanged". @@ +47,5 @@ > "however, the key '%s' " % repo_name + \ > "is not found in it." > > + # For some (but not all) platforms and repos, -pgo is explicit in > + # the trigger but not in the shortname Could you please give an example? @@ +50,5 @@ > + # For some (but not all) platforms and repos, -pgo is explicit in > + # the trigger but not in the shortname > + SUFFIXES = ['-opt-unittest', '-unittest', '-talos', '-pgo'] > + > + if buildername not in buildername_to_trigger: Could you please add a note?

FYI for all, the tests will not yet pass since we are not yet generating a new version of allthethings.json. It is still pending Releng deployment. I've asked catlee on IRC to update it for us.

Comment on attachment 8565760 [details] [diff] [review] test_platforms.diff Review of attachment 8565760 [details] [diff] [review]: ----------------------------------------------------------------- Excellent job. Again, I have commited this into the bisection tree. Could you please follow up with the comments I addressed? ::: test/test_platforms.py @@ +19,5 @@ > + with open(filepath, 'r') as f: > + reference_builders_info = json.load(f) > + > + # Build jobs should return themselves > + build_jobs = set(reference_builders_info.values()) I think this would read better like this - what do you think? # The values of all the builds can be obtained by using .values() on the # data from test_platforms.json. When asking associated_build_job() for a builder # we return itself. We add known build jobs to reference_builders_info in such manner. @@ +23,5 @@ > + build_jobs = set(reference_builders_info.values()) > + for build_job in build_jobs: > + reference_builders_info[build_job] = build_job > + > + builders_data = mozci.sources.allthethings._fetch_json()['builders'] Can you please call this "latest_builders"? Could you please call mozci.query_builders()? @@ +27,5 @@ > + builders_data = mozci.sources.allthethings._fetch_json()['builders'] > + > + tests = [] > + for builder in reference_builders_info.keys(): > + properties = builders_data[builder]['properties'] I was thinking about how we could detect if new builders are added to allthethings.json and warn that we have them missing. Currently we wouldn't know. Or maybe run a script to generate a newer test_platforms.json and we can simply grab it and check it in. What do you think? Do you have such script to generate a newer test_platforms.json? We should check it in as test/generate_test_platforms_json.py @@ +28,5 @@ > + > + tests = [] > + for builder in reference_builders_info.keys(): > + properties = builders_data[builder]['properties'] > + # If we can't guess a repo_name we can't test associated_build_jobs When would we hit this case? Would we silently start missing certain jobs? @@ +34,5 @@ > + repo_name = properties['repo_path'].split('/')[-1] > + except: > + continue > + > + expected = reference_builders_info[builder] Can you please change it to "expected_buildername"? @@ +40,5 @@ > + > + return tests > + > + > +@pytest.mark.parametrize("builder,repo_name,expected", complex_data()) You did such a beautiful job in here :) Now I have learned more thanks to you!

I have deleted the bisection branch. Please use the master branch. Your changes are there now.

This has long been accomplished by Alice! Thanks!