[snowpatch] [PATCH] Initial support for testing from multiple base branches

Andrew Donnellan andrew.donnellan at au1.ibm.com
Wed Apr 6 15:02:56 AEST 2016


On 01/04/16 20:46, Russell Currey wrote:
>> +    for branch_name in project.branches.clone() {
>> +        let tag = tag.clone();
>
> We need to have a different tag per branch.  Otherwise, branches are going
> to be overridden for everything we try and merge the patch into.  Now this
> shouldn't be an issue from what I see, since we wait for the test to finish
> before starting the next one (right?). However, we're not deleting remote
> branches meaning having a remote history of the stuff we test is good, and
> in future I would like it if we kicked off a job for a branch and didn't
> block on it completing, and this would be required for that (since the
> remote branches would conflict).
>
> Appending the branch on the end should be enough, whether we only want to
> do this if test_all_branches is set is up to you.

ACK.


>> +        match output {
>> +            Ok(_) => {
>> +                results.push(TestResult {
>> +                    test_name: "apply_patch".to_string(),
>> +                    state: TestState::SUCCESS.string(),
>> +                    url: None,
>> +                    summary: Some("Successfully applied".to_string()),
>> +                });
>> +            },
>> +            Err(_) => {
>> +                // It didn't apply.  No need to bother testing.
>> +                results.push(TestResult {
>> +                    test_name: "apply_patch".to_string(),
>> +                    state: TestState::FAILURE.string(),
>
> If we're supporting multiple branches, then failing to apply shouldn't be a
> FAILURE, but a WARNING.  Lots of patches aren't going to apply to every
> branch.  Maybe set to FAILURE if test_all_branches is false?
>
> Another issue is what happens if test_all_branches is true and it fails to
> apply to every branch.  Maybe we can do a check at the end such that if
> everything in the Vec<TestResult> is WARNING (i.e. no successes to mark as
> success, and no failures to mark as failure), we can add a new one with
> FAILURE to make it very clear the patch sucks.

s/is true/is false/, s/is false/is true/

I'll think about the best way to do this. Also, the current results 
returned don't tell you at all which branch you failed on. I should fix 
that.

Will submit V2.

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited



More information about the snowpatch mailing list