Friday, January 26, 2018

Do you treat SQL code as a first class citizen?

Too many times I have seen developers treating SQL code and the database itself as a second class citizen. They would go into a production database and make stored procedure or table structural changes directly in production. The database was an afterthought and not carefully crafted and discussed the way that their parts of the product coded in their language of choice was, be it C#, Java, C++, etc. In my opinion this is wrong, it leads to bugs and technical debt and for medium to large systems it often leads to performance issues. It is similar in some ways to treating test code as a second class citizen, where one then spends an inordinate amount of time looking for bugs in production code that are actually in the test code because the same care wasn't taken with the test code as the production code, but that is a soap box for another post.

What do I mean as SQL code as a first or second class citizen? What I mean by that is that you give it the same level of care and attention as the rest of the code in your system.

If you treat SQL code as a first class citizen, I expect to see the following:
  • Just as you treat the source code in source control as the source of truth you treat the database files in source control as the source of truth. It is very telling when a team treats a database as the source of truth. When this is done in most cases, the team cannot recreate the database from scratch, which makes integration and system testing more difficult. When a team is constantly adjusting the files in the source control to match a database that is an indication that the source control is not being treated as the source of truth. Would you adjust binaries and then go make the source code match?
  • Just as source code for the other languages you code in are stored in source control, so are the SQL files. This is regardless of if you deploy the database as a model or via files, you ensure that the artifacts needed to deploy the database are stored in source control. These may be a model or individual SQL file.
  • Just as the team takes the time to properly chose algorithms and data structures in their primary language, they make sure to do the same in SQL code. They make sure that the correct key and constraints are in place, using the 
  • Just as you author a deployment system for your code, you do the same for the database, in some case they can be part of the same deployment process.
  • Just as your team has coding standards for your primary language(s), you also have codding standards for SQL. (You do have standards right?) These standards should all be at the same level of detail.
  • Just as your team code reviews their other code, they review SQL and apply the same rigor to those reviews.
I am sure there are more comparisons, but as you can see it is simply a matter of treating the database in all aspects with as much care as we treat our other languages, from source code to design to deployment.

Please feel free to share your thoughts. 



Thursday, January 11, 2018

TFS2017 - The build server is running more tests than we wrote. Wait, what?!?

There are a few cases in which the build server will run more tests than exist in a project. In all cases it is because one or more tests are being run more than once. This most often is easily understood and happens due to the test code being compiled more than once. This can happen when compiling for:
  • Multiple configurations such as Debug and Release
  • Multiple frameworks, see my previous post
All of these cases are easy to detect from looking at the build definition alone. I recently ran into a case where a team wrote 119 tests and 135 where being run during the build. They were building a single configuration and targeting a single framework so the code was only compiled once. This anomaly was therefore not due to any of the situations mentioned above.

To start to debug this situation we downloaded the trx file from the TFS Server for one of their builds and noticed there were 16 tests with the same name and same ID. Looking through the testing step output from the build, we saw the same set of 16 tests duplicated, so now we knew which 16 were being run twice but we still needed to determine the cause of the duplication. 

Looking at the code provided the next clue. The 16 tests were all in one test assembly and they were the only tests in that assembly. Also in that assembly were some abstract classes used by the tests.

Looking at the build output we found that the assembly containing those 16 tests was in two directories, which explained why those 16 tests were being re-run, because the testing step was configured to find any assembly that matched the pattern *Test.dll and execute it, so it found both instances of the assembly and executed the tests in each copy.

The final piece of the puzzle came when we found that the dll with the 16 duplicated tests in it was referenced by another unit test project. On compilation this caused that dll to be in its bin folder and in the bin folder of the project that referenced it. Those abstract classes I mentioned earlier were used by both projects. We did a bit of refactoring, pulled the common non-test code into a new assembly and the duplicate tests no longer showed up in the build.

The moral of the story: Assemblies containing unit tests should not reference other assemblies with Unit Tests. Be mindful of dependencies. While it is easy to reference other projects, it isn't always a good idea. This reminds me of a project that had circular build dependencies, that that is a story for another time.

 

Wednesday, January 3, 2018

.NetCore - How to enable Code Coverage in Visual Studio 2017

In my previous post I discussed how to enable code analysis in .Net Core. One of the other tools that I used in the .Net Framework world was Code Coverage. I like to have a sense of how much code my unit tests cover. This is one of many metrics I like to look at to get a sense of code quality. 

Creating a Unit Test project is simple and documented many places so I won't duplicate that here. What I want to cover is what you need to do to a .Net Core project to enable Code Coverage results to be displayed. Code Coverage requires a classic Windows PDB file as opposed to the new format (click here for details of why a new format was created.) To do this you need to add the DebugType tag to the project with a value of Full

<DebugType>Full</DebugType>

This will cause the project to generate a classic Windows pdb so that the Code Coverage tools in Visual Studio will work correctly. It is often, almost always, useful to also create a runsettings file so that the test project doesn't display in the code coverage report and so that you can tweak it to only show your code. Details on creating a runsettings file, including a sample can be found here

Lastly, there are a lot of posts on the internet about the evils of a full pdb in release mode (optimized) builds. This StackOverflow post incorrectly states that there is a performance impact in one of the comments. In Native code and in .Net Framework 4.0 and above and in .Net Core there is NO performance impact of creating a full pdb. Here is a Wintellect Post by John Robbins with the details. This Stack Overflow post sums the article up nicely, it is what led me to John Robbin's Post.

.NetCore - How to enable Code Analysis with Stylecop.analyzers in Visual Studio 2017

Pre .Net Core enabling code analysis for a C# project only required clicking a checkbox on the project settings. Now with .Net Core and the Roslyn compiler there are more steps to enable code analysis, but there are also more options for analyzers. 

The analyzer I like is StyleCop. In this post I will show you how to setup StyleCop and configure it. It is well documented in the GitHub repo, but there were a few things that I noticed I needed to do that weren't mentioned there.

To Start with, assuming you have .Net Core project that you want to add code analysis to, you need to add the NuGet package for StyleCop.Analyzers version 1.0.2 as of this writing.

This line will be added to the csproj file
<PackageReference Include="StyleCop.Analyzers" Version="1.0.2" />


If you are developing a library, then unless you want all consumers of your library to also require that package you need to make the following change


<PackageReference Include="StyleCop.Analyzers" Version="1.0.2" PrivateAssets="All" />



As per the csproj reference this says that the StyleCop.Analyzers package should be used by the current project, but not be required as a dependency outside of the current project.



Now when you compile you will get warnings that start with "SA" such as 



SA1101: Prefix local calls with this 



To configure StyleCop Analyzers, you need to add stylecop.json to each project. As per the documentation the easy way to do this is to invoke a code fix on SA1633


This will generate the default file. If you have more than one project in the solution, I suggest you move the generated file to the same folder as the .sln file, and then share the one file among all the projects so that they all have the same StyleCop configuration. 



You will need to add the file to each project, add it as a link to avoid making copies of the file, which was the point of having a single file.


For Visual Studio 2015 and 2017 you need to enable Stylecop.json. For Visual Studio 201, in each project right click on stylecop.json, select properties and in the properties page for the file set the build action to C# analyzer additional file.


Make sure to add stylecop.json to source control. Stylecop.json allows you to control the stylecop analyzer rule specifics. I always add the following at a bare minimum to the generated file. 

"indentation": { "indentationSize": 4, "tabSize": 4, "useTabs": true }, "orderingRules": { "systemUsingDirectivesFirst": true, "blankLinesBetweenUsingGroups": "allow", "usingDirectivesPlacement": "outsideNamespace" }

For more information on stylecop.json see the StyleCop.json documentation

Now that code analysis is configured you should be seeing code analysis issues in the Error List in Visual Studio. Each code analysis issue is a link that will take you to a page that describes the issue and what causes it. We have a number of options for how to address the issues.


  1. We fix the issue - change the code to address the problem
  2. We suppress it in source using #pragma warnings. This can be done by right clicking on the item in the Error List and selecting Supress | In Source
  3. We suppress it in a Suppression file. Visual Studio will create a file in the project named GlobalSuppression.cs if it does not already exist and use System.Diagnostics.CodeAnalysis.SuppressMessage() to suppress that instance of the message
  4. Use a ruleset file to turn off the rule. This one takes a bit more explanation. Now that 

Using a ruleset file to manage code analysis rules

To enable the use of a ruleset file you need to add a CodeAnalysisRuleSet tag to the csproj. That tag should contain a relative path to a .ruleset file, which is just an xml file. I like to put the .ruleset file in the same directory as stylecop.json and the .sln file.
<CodeAnalysisRuleSet>..\..\CodeAnalysisRules.ruleset</CodeAnalysisRuleSet>


It is easiest, in my opinion if you edit the ruleset file as text. You can use the file to hide rules that you don't want to apply to your code. Here is a sample ruleset file.


<?xml version="1.0" encoding="utf-8"?> <RuleSet Name="My Code Analysis RuleSet" Description="This rule set contains all rules that I want to have enforced for C# programs." ToolsVersion="14.0">   <IncludeAll Action="Warning" />   <Rules AnalyzerId="StyleCop.Analyzers" RuleNamespace="StyleCop.Analyzers">     <Rule Id="SA1101" Action="None" />     <Rule Id="SA1623" Action="None" />     <Rule Id="SA1633" Action="None" />
  </Rules> </RuleSet>

In this ruleset I have disabled rules SA1101, SA1623 and SA1633. I set the rules to None, there are other options as listed here. As is pointed out on that page there is a subtle difference between marking a rule as Hidden vs None. "When a diagnostic rule is set to Hidden instances of that rule are still created, but aren't displayed to the user by default. However, the host process can access these diagnostics and take some host-specific action. For example, Visual Studio uses a combination of Hidden diagnostics and a custom editor extension to "grey out" dead or unnecessary code in the editor. A diagnostic rule set to None, however, is effectively turned off. Instances of this diagnostic may not be produced at all; even if they are suppressed rather than being made available to the host."

Now you know how to enable Code Analysis for .Net Core project to improve the quality of your code.

The 2024 State of DevOps Report and the Importance of Internal Development Platforms

On the State of DevOps Report The State of DevOps Report, published annually by the DevOps Research and Assessment (DORA) team, has been a c...