MicrosoftPremier / VstsExtensions

Documentation and issue tracking for Microsoft Premier Services Visual Studio Team Services Extensions
MIT License
57 stars 14 forks source link

Can't find previous count if used in build in pull request #31

Closed MikeWilliams-UK closed 4 years ago

MikeWilliams-UK commented 5 years ago

I have a git repository set up. I have protected the master brach by setting a build policy which includes running a build.

This build pipeline has a build quality check in it, which never finds the number of warnings from master. If another push is done it sees the count from the last build of the pr, but never from master.

Hence after a successful pr, if I re run the build it can fail if the merge done as part of the pr has introduced more warning.

MikeWilliams-UK commented 5 years ago

See below image

MikeWilliams-UK commented 5 years ago

Have I got a setting wrong somewhere?

ReneSchumacher commented 5 years ago

Hi Mike,

this is most probably a configuration issue. By default the task always compares to the previous build that ran for the same branch. PRs use a special branch that is created to simulate the merge from your topic branch into your target branch. Thus, the task only finds warnings from that special branch.

In order to check the warnings from master, you need to set the correct baseline branch like this: image

Select the build definition, the repository and the branch in the baseline section. That should do the trick. Let me know if there's anything else you need.

René

DigitalFlow commented 5 years ago

Hi @ReneSchumacher,

thanks for your answer. I think I'm having the same issue as @MikeWilliams-UK. I thought a little about it and it seems as if we were only able to tackle this problem by changing our branch creation workflow. This issue does not only occur in PRs, but also in normal (new) branches.

If we have a develop branch with a coverage of 70% and we create a feature branch out of it, there are two possibilities:

So to me it seems as if the fix for using the previous build comparison would be to push all new branches directly before the first commit and to directly create a PR as well to ensure that the initial coverage for this branch is present with the same value. Any thoughts on this @ReneSchumacher?

Thanks a lot for this extension, my team and I like it a lot.

Kind Regards, Florian

Edit: Turns out you can't create a PR from one branch to another if they don't have any differences, you would have to change a none code file first.

ReneSchumacher commented 5 years ago

Hi @DigitalFlow

I'm glad you like the extension. In fact, most of the teams here at Microsoft like it too and the Azure DevOps testing team is probably going to move some of the functionality into the product. I still hope my extension can stay relevant, though :)

Your description is correct. Since the task looks for builds that ran on the same branch, every build on a new branch will become a new baseline unless you configure a different basline branch in the task. We've discussed this internally and think that we should be able to provide a better solution in an upcoming version. Instead of limiting the policy to the current branch, we would simply walk the history graph back in time until we find the parent branch that has a build. With this new bahavior, the first build on a topic branch should look for the previous build on the parent branch by default. If the policy succeeds, that build becomes the correct new baseline for future builds on the topic branch. Otherwise the next build would again look for the last build on the parent branch.

I still have to flesh out the details for this, but it feels like a better way of handling baselines. Of course, we'd still have the option to configure a fixed baseline for cases where customers need to compare to builds from a specific branch or a different build definition. Think that might be helpful?

René

DigitalFlow commented 5 years ago

Hi @ReneSchumacher,

yes, that will definetly be helpful. Sounds like a very good approach 👍

Kind Regards, Florian

MikeWilliams-UK commented 5 years ago

Hi @ReneSchumacher thanks for the tip on the settings that worked a charm.

ReneSchumacher commented 5 years ago

I am closing this issue since it was resolved with the configuration change. With one of the next updates we will try to incorporate your feedback and automatically search for a suitable baseline build along the branch hierarchy.

gerwinjansen commented 4 years ago

Coming back to the original problem of pull requests: I'm having a master branch and maintenance branches for older versions. All of them are protected, so require a PR to accept changes.

I end up changing the azure-pipelines.yml every time I create a new protected branch and set the baseBranchRef.

What if the default behavior (when empty) is:

  1. Check if variable System.PullRequest.TargetBranch is available and use the latest build of that branch
  2. Check if a previous build of the same branch is available and use it
  3. Check if a previous build of a parent branch is available and use it

Doing this, most scenario's will work out-of-the-box, and the baseBranchRef can be used for exceptional scenario's. Might even be possible to remove this configuration option completely.

ReneSchumacher commented 4 years ago

Hey @gerwinjansen,

thanks for bringing this up again. This request still sits on our backlog because we couldn't come up with a solution that works for all cases. No. 1 and 2 in your list work perfectly fine (no. 2 is the current behavior and no. 1 would be a quick addition). However, no. 3 won't work.

Even though many people think in parent and child branches, there is nothing like that in Git. A branch is simply a reference to a particular commit and doesn't have any information about its "natural" parent (note: I'm not talking about upstream/tracking branches here as they don't play a role in our scenario). Thus, the question we've been racking our brains on remains: how should we pick an appropriate baseline branch.

Imagine you have a history of A-B-C-D with master pointing to B and topic1 pointing to D. In that case it would be very simple to choose master as the baseline (just walking the history graph until we find master). However, imagine that someone now creates branch topic2 pointing to C. If we walk the graph again, our "natural" parent would all of a sudden be topic2, even though that isn't the "correct" parent of topic1.

There are many scenarios like this and we've been thinking about adding parameters like a branch match pattern or something similar that would help us pick the right branch but couldn't decide which way to go. I believe it's time to split that problem up and just deliver a first improvement by picking the PR target branch when we cannot find a baseline for the current branch. That would probably be good enough for most users.

Btw: you can achieve a similar thing with the current version of the task. Just set the value of baseBranchRef to System.PullRequest.TargetBranch as you mentioned. If the build does not run in the context of a PR, the variable will be undefined and the task falls back to the current branch. In a PR context, though, it will automatically look for builds on the PR target branch. It's not exactly the same behavior we talked about but it's better than changing the YAML pipeline every time.

ReneSchumacher commented 4 years ago

Closing this issue again. We have just release v7.1.0 of the extension which has the requested behavior.