I recently had the opportunity to create a build that would fail if certain file types did not have a passing code review in TFS. For those who would like to do something similar, I’ll detail the way I accomplished it.
Handling Code Reviews
Obviously, the first step in this process is having a way to keep track of the code reviews themselves. For that, I used the Scrum 2.0 template that comes with TFS2012. For those not using TFS2012 yet, you can import the Code Review Request and Code Review Response work item types in to a TFS2010. If you’re not using Visual Studio 2012, then you’ll need to modify the layouts of those work item types so that all the inputs aren’t read only. Once this part is done, this will associate a change set to a code review.
Creating the Custom Build Activity
The custom activity will need a few parameters to work its magic. First, it’ll need an instance of IBuildDetail to fail the build if there is non-reviewed code. An instance of a team project collection is needed as well – it’ll be used to get the work item store to check for code reviews. It’ll also need a list of change sets in the build to check. Lastly, it’ll take what file types to check (for example, “.sql,.cs” or blank for all) and a Boolean specifying if the build should be failed or not (it’s a lot easier to change that variable than to remove the activity from the build if a situation arises where the code review can be skipped). Here’s what the parameters look like:
///<summary>
/// Gets or sets the build detail
///</summary>
[RequiredArgument]
[Browsable(true)]
public InArgument<IBuildDetail> BuildDetail { get; set; }
///<summary>
/// Gets or sets the team project collection. Use Microsoft's GetTeamProjectCollection activity.
///</summary>
[RequiredArgument]
[Browsable(true)]
public InArgument<TfsTeamProjectCollection> TeamProjectCollection { get; set; }
///<summary>
/// Gets or sets the changesets to check for code reviews
///</summary>
[RequiredArgument]
[Browsable(true)]
public InArgument<IList<Changeset>> Changesets { get; set; }
///<summary>
/// Gets or sets the comma separated list of files types to check (for example, ".sql,.cs"). An
/// empty string means all files require review.
///</summary>
[Browsable(true)]
public InArgument<string> CommaSeparatedFileTypes { get; set; }
///<summary>
/// Gets or sets a value indicating whether the build will fail if there is non-reviewed code
/// matching the specified file types.
///</summary>
[Browsable(true)]
[RequiredArgument]
public InArgument<bool> FailBuildOnNonReviewedCode { get; set; }
An Overview of the Process
At a high level, the execute method of the custom activity will check all the change sets to see if there is any code without a passing code review, output the files that don’t have a passing review, and optionally fail the build. Here’s what the execute method looks like:
/// <summary>
/// Performs the execution of the activity
/// </summary>
/// <param name="context">The execution context under which the activity executes</param>
protected override void Execute(CodeActivityContext context)
{
#region Validate arguments and parameters
if (context == null) { throw new ArgumentNullException("context"); }
TfsTeamProjectCollection teamProjectCollection = context.GetValue(TeamProjectCollection);
if (teamProjectCollection == null) { throw new ArgumentException("Team Project Collection cannot be null."); }
IBuildDetail buildDetail = context.GetValue(BuildDetail);
if (buildDetail == null) { throw new ArgumentException("Build Detail cannot be null."); }
IList<Changeset> changesets = context.GetValue(Changesets);
if (changesets == null) { throw new ArgumentException("Changesets cannot be null."); }
#endregion
ICollection<string> filesTypesRequiringReview = ParseFileTypesRequiringReview(context.GetValue(CommaSeparatedFileTypes));
bool failBuildOnNonReviewedCode = context.GetValue(FailBuildOnNonReviewedCode);
WorkItemStore workItemStore = teamProjectCollection.GetService<WorkItemStore>();
IEnumerable<Change> nonReviewedChanges = DoesBuildHaveNonReviewedCode(workItemStore, changesets, filesTypesRequiringReview);
bool anyNonReviewedChangesets = false;
foreach (Change nonReviewedChange in nonReviewedChanges)
{
anyNonReviewedChangesets = true;
HandleChangeWithoutPassingReview(context, failBuildOnNonReviewedCode, nonReviewedChange);
}
if (anyNonReviewedChangesets && failBuildOnNonReviewedCode)
{
buildDetail.Status = BuildStatus.Failed;
}
}
Checking for Non Reviewed Code
To check for non-reviewed code, the code will loop through all the change sets and check each change to see if the file matches the file types requiring review. If it does, it then checks to see if there’s a passing code review. If the change doesn’t have a passing review, it’s returned so the build can output all the files that still need a review. Here’s the code:
/// <summary>
/// Determines if the build has any changesets that have not been reviewed
/// </summary>
/// <param name="workItemStore">Work item store</param>
/// <param name="changesets">Changesets</param>
/// <param name="filesTypesRequiringReview">File types requiring review (empty collection means all)</param>
/// <returns>Changes without a passing review</returns>
private IEnumerable<Change> DoesBuildHaveNonReviewedCode(WorkItemStore workItemStore, IList<Changeset> changesets, ICollection<string> filesTypesRequiringReview)
{
foreach (Changeset changeset in changesets)
{
// The changeset object passed in may not contain the associated changes (depending on how this is executed - workflow versus unit test)
Change[] changes = changeset.Changes.Length > 0 ? changeset.Changes : changeset.VersionControlServer.GetChangesForChangeset(changeset.ChangesetId, false, int.MaxValue, null);
foreach (Change change in changes)
{
bool matchingFileToCheck = DoesChangeRequireReview(change, filesTypesRequiringReview);
if (matchingFileToCheck)
{
bool hasPassingCodeReview = DoesChangesetHavePassingReview(workItemStore, changeset);
if (!hasPassingCodeReview)
{
yield return change;
}
}
}
}
}
Determining if the Change Requires a Review
A change requires a review if the file type matches the file types passed in (or if none were passed in, then every file requires a review).
/// <summary>
/// Determines if the change requires review
/// </summary>
/// <param name="change">Change</param>
/// <param name="fileTypesRequiringReview">File types requiring review</param>
/// <returns>True if a passing review is required, else false</returns>
private static bool DoesChangeRequireReview(Change change, ICollection<string> fileTypesRequiringReview)
{
bool matchingFileToCheck = fileTypesRequiringReview.Count == 0;
foreach (string fileType in fileTypesRequiringReview)
{
if (change.Item.ServerItem.EndsWith(fileType, StringComparison.CurrentCultureIgnoreCase))
{
matchingFileToCheck = true;
}
}
return matchingFileToCheck;
}
Determining if the Change Set Requires a Review
This is the crux of the code. To determine if the change set has a passing review, we query the work item store based on the change set ID. In Visual Studio / TFS 2012, code reviews aren’t linked to the change set as one might expect. This is because code reviews can be associated with a change set or a shelve set. As a result, the change set objects passed in to the code won’t have the associated code review work items. Instead, we have to check the context field. Here’s the code:
/// <summary>
/// Determines if the changeset has a passing code review associated with it
/// </summary>
/// <param name="workItemStore">Work item store</param>
/// <param name="changeset">Changeset</param>
/// <returns>True if the changeset has a passing review, else false</returns>
private static bool DoesChangesetHavePassingReview(WorkItemStore workItemStore, Changeset changeset)
{
Query query = new Query(workItemStore,
string.Format(CultureInfo.InvariantCulture,
"SELECT [Id] FROM WorkItemLinks WHERE "
+ "[Source].[System.WorkItemType] = 'Code Review Request' "
+ "AND [Source].[Microsoft.VSTS.CodeReview.Context] = '{0}' "
+ "AND [System.Links.LinkType] = 'Child' "
+ "AND [Target].[System.WorkItemType] = 'Code Review Response' "
+ "AND ([Target].[Microsoft.VSTS.CodeReview.ClosedStatusCode] = 1 OR [Target].[Microsoft.VSTS.CodeReview.ClosedStatusCode] = 2) " // 1 = Looks Good, 2 = With Comments
+ "mode(MustContain)",
changeset.ChangesetId)
);
return query.RunCountQuery() > 0;
}
Outputting Files without a Review
If the build fails because there is code without a passing review, it’s extremely helpful to know what files still need to be reviewed. The DoesBuildHaveNonReviewedCode method returns the list of non-reviewed changes. Depending on whether or not we’ll be failing the build, the code outputs those files accordingly:
/// <summary>
/// Takes the appropriate actions when a changeset does not have a passing review
/// </summary>
/// <param name="context">Context</param>
/// <param name="treatAsError">True if the message should be treated as an error, else it will be a warning</param>
/// <param name="change">Change</param>
private static void HandleChangeWithoutPassingReview(CodeActivityContext context, bool treatAsError, Change change)
{
if (treatAsError)
{
WriteBuildError(context, string.Format(CultureInfo.CurrentCulture, UnreviewedFileMessageTemplate, change.Item.ServerItem));
}
else
{
WriteBuildWarning(context, string.Format(CultureInfo.CurrentCulture, UnreviewedFileMessageTemplate, change.Item.ServerItem));
}
}
Adding the Activity to the Build Template
Next we need to place the custom activity in our build definition. The “Associate Changesets and Work Items” activity produces a list of change sets in the build. We’ll need that along with a GetTeamProjectCollection activity. Here’s the placement:
Passed Parameters
I made file types and Boolean to fail the build with non-reviewed code parameters. This way I can modified what file types require review and if the build should fail or not without having to update the XAML.
Results
Here’s what it looks like when run the build when it’s set to fail on non-reviewed code:
And here’s what it looks like when you treat it as a warning:
