Monday, December 28, 2009

Unit Testing 101 Guidelines Part 4

Also see See also Unit Testing Guidelines Terminology, Part1, Part2, Part3, Part4


Standard Practices & Guidelines

Code Coverage Levels

·         DO have at least 60% code coverage with any new Microsoft.Net code written.

·         CONSIDER aiming for 80% + (Remember it is not possibly to unit test ALL code, so don't beat yourself up if you can't get to 100%).

·         DO analyse your code using the Code Coverage panel in Visual Studio. This will show you the code paths not tested.

Structure of Test Fixtures and Projects

·         DO write at least one Test Fixture per production class.

·         CONSIDER writing more than one Test Fixture per production class. Test Fixtures with fewer methods and with clear themes are easier to maintain.

·         CONSIDER creating one test project per Visual Studio solution, not one test project per production project.

·         DO NOT add references to test libraries in the production projects. (ie NunitFramework.dll, RhinoMocks.dll, YourTestProject.dll etc).

·         DO give your test fixtures strong names with no acronyms.  Better it be long and descriptive than short and cryptic.

·         DO put Integration Tests and Unit Tests in different namespaces within your test project. This will make it easier for people to bypass the longer larger integration tests if they choose to do so.

·         DO NOT inter-mingle unit tests and integration tests in the same test fixture.

·         AVOID using constructors in test fixtures. Use methods decorated with the [Setup] attribute instead.

·         DO NOT use IDisposable or destructors in test fixtures instead use the [Teardown] attribute on your clean up method.

·         DO put all your accessors in a sub-folder and different namespace within your test project.

·         DO put all your mocks and stubs in a sub-folder and different namespace within your test project.

Single Test Design

·         CONSIDER the theme of your test.  Each test should be testing a specific use case or code path. Better to the have lots of simple tests than few complex difficult to maintain tests.

·         AVOID designing your tests to be slower than 500ms each.

·         CONSIDER optimising until they are running in less than 100ms each.

·         CONSIDER keeping your tests to 50 lines of code or less.

·         DO NOT let your test exceed 100 lines of code.

·         DO give your tests strong names with no acronyms.  Better it be long and descriptive than short and cryptic.

·         CONSIDER giving all your tests descriptions using the [Test] attribute. For example: [Test(Description = “My test that tests the something method and looks for this...”)].

·         DO NOT write tests that could affect the running of other parallel tests. For example avoid using class level or static variables, except if they are readonly.  Tests should always be able to run in parallel; in fact most tools do this by default (Resharper and NUnit Gui included).

·         DO NOT write tests that fail periodically and pass sometimes. Tests should always be consistent.

·         AVOID writing tests that access more than one public method or entry point into a class. Write more tests.

·         DO always add the [Timeout] attribute to all your tests.  Give it at most a 500ms timeout. This will prevent failing tests waiting for internal timeouts to expire to fail the test. (You will need to remove the attribute to debug the test).

·         CONSIDER using a Mock framework like RhinoMocks to automatically generate your mocks and stubs. Writing them by hand is tedious and time consuming.

·         CONSIDER extensively commenting your tests.  Tests are commonly used for explaining how to use a particular class.  It also increases maintainability of the test.

·         DO use a wide range of input values for a test. Always test fringe data like null, empty, zero, and negatives where appropriate.

·         CONSIDER using the [Values] attribute rather than copying and pasting the same test over and over but with different input data for the targeted method.

·         AVOID using try...catch to catch expected exceptions in tests.  For example if you want to ensure a NullArgumentException is thrown when a consumer tries to pass null to a method, use the [ExpectedException] attribute rather than wrapping the test in try...catch.

·         DO test all exceptions thrown by a code path. If the method being tested has code that specifically throws exceptions, write tests for these use cases.

·         DO check the state of the class as well as return values of functions.  A function could change the fields within the class as well as return a value.

Dependencies

·         DO mock all references the class under test has to other classes.  Exceptions are listed below in this section.

·         AVOID mocking/stubbing dependencies of type ICommand.  Commands are one of the few very simple standard objects that can be exempted.  This only applies to Commands of type RoutedCommand and RelayCommand. If you have a custom implementation then it will need to be mocked.

·         AVOID mocking/stubbing dependencies that are Data Transfer Objects (DTOs).  These are objects that are used to transfer data to and from a WCF service.  These objects are basically structs and do not contain any logic.

·         CONSIDER using Strict mocks when using Rhino Mocks whenever possible as they catch a much wider range of issues with much less code.

·         AVOID using a string based private accessor. Prefer instead a strongly typed private accessor like those generated by the PrivateAccessorGenerator class.

What to test

·         DO test all public methods of a class. Most often the non-public members will be called by the public members. If the non-public members are not being covered by testing only the public members consider testing them too.

·         CONSIDER testing all paths through a method of code. Use the Code Coverage analysis tool inside Visual Studio to show you the paths currently not being tested (they will be highlighted in pink).

·         DO test all main points of entry into the class. An entry point might be a public method, or protected or internal. 

·         DO test the constructors of a class to ensure the class is in a predictable defined state upon instantiation.

·         CONSIDER testing internal and protected members if the public methods of a class do not call all internal and protected methods.

·         DO NOT test properties, fields, structs, and enums. These members should not contain any logic.  The only exception to this rule is the properties who raise the INotifyPropertyChanged.PropertyChanged event.

·         DO test properties that intentionally throw exceptions.  This technique is sometimes used to validate property setters in WPF. These setters should be tested with some known valid and invalid cases.

Design for Testing

·         DO NOT write tests that require user intervention.

·         DO NOT add test code or test concerns to production code. Unit testing is supposed to be unobtrusive into the production code and leave no footprint on the production code.  This means no test libraries should be referenced by production assemblies. There should be no if statements or compiler switches for testing versus production. By looking at the production code you should not be able to tell unit tests are being used against a class.

·         DO use interfaces for references to other classes.

·         CONSIDER using a basic abstract class instead of an interface but prefer interfaces.

·         DO use the following dependency injection techniques in this order or preference:
1)            Interfaced properties that use a backing field, that if null creates the production instance of that type.  A private setter can be used to inject a test mock.
private IMessageBoxService backingMessageBoxService;
public IMessageBoxService MessageBox {
    get {
        return this.backingMessageBoxService ?? (this.MessageBox = new WpfMessageBoxService());
        }

        private set {
            this.backingMessageBoxService = value;
        }
    }
}
This is best when only a test mock and a production type are used. If there could be more then rather use an Object Factory to get the instance.

2)            Constructor injection. The number of constructor arguments should be no more than 3 or 4.

public FormController(IContactDataService contactDataService) { … }

3)            Use an Object Factory to get an instance of an interface.

private IMessageBoxService backingMessageBoxService;
public IMessageBoxService MessageBoxService {
    get {
        if (this.backingMessageBoxService == null) {
            this.backingMessageBoxService = StructureMap.ObjectFactory.Container.GetInstance<IMessageBoxService>();
        }
 
        return this.backingMessageBoxService;
        }
    }
}




·         DO NOT use property injection. It is very slow in comparison to constructor injection and object factory look-up.

·         DO NOT call the static class MessageBox directly. Instead see this example: Message Box Usage

·         DO NOT show Windows directly. Ie Do not call Window.Show().  Instead see this example:
·         Showing Another Window From a Controller
·         AVOID static properties or fields in a class. Rather ask the object factory to return you a singleton reference.  The key piece is the .Singleton() method call.
ObjectFactory.Initialize(init => {
    init.For<ITestSingleton>().Singleton().Use<TestSingleton>();
});
var singleton = ObjectFactory.GetInstance<ITestSingleton>();

In this example the same instance will be returned whenever someone asks for an instance of ITestSingleton.

Bugs

·         DO write at least one test every time you find a bug or are given a bug report to resolve. This ensures the exact same bug cannot reoccur. The test should reproduce the bug and fail.  You can then write code to make the test pass. Ensure your test(s) covers all scenarios documented in the bug report.

·         CONSIDER writing more tests if you can see more than one way the bug could manifest itself.

·         DO write only the code necessary to make your tests pass.

Unit Testing 101 Guidelines Part 3

See also Unit Testing Guidelines Terminology, Part1, Part2, Part3, Part4

Design for Unit Testing

Production code must be designed from the start to be unit testable. Trying to retro-fit unit tests after a software component is complete will mean its highly unlikely you can focus your unit tests on one method of code and not invoke other production components.

Walk Through Example

The main goal of unit testing is to isolate the subject under test from all other dependencies. Consider the following code where the FormController is the intended subject under test. 



Ideally we need to replace all external dependencies with mocks or stubs. Thereby controlling the input of the class we can predicate the output.

The constructor takes the IContactDataService interface as input argument.  Because this interface is essentially an input for this class we need to replace the real implementation with a fake one that the test can control. We can create a new implementation of this interface that can be used instead of the real production implementation. If this was not an interface we would be unable to replace this dependency.  

Imagine this constructor argument was not an interface and was a normal class type. This would probably mean we would be using a class that will try to communicate with some external data service, given its name is IContactDataService. For this test to work we would need a fully operational service; and if the service failed our test would fail. This is unacceptable; we do not want our test to fail unless the actual code directly inside the FormController class fails. Fortunately this constructor argument is an interface, therefore it can be replaced.

Now look at the MessageBox property.  This property has a return type of IMessageBoxService.  At some point the class is going to access this property, probably to interact with a message box dialog.  If this property was designed to call the message box functionality in WPF directly this would mean an OS modal message box could pop up at anytime and stall the test until somebody clicks “Ok”.  Fortunately we can again provide our own test implementation of this interface and check that the class is invoking the appropriate message box method and passing in the correct data.

The FormController class has three commands it uses to expose functions to the Xaml view. Generally speaking I would not bother mocking an ICommand, seeing as it is an extremely common mechanism and there is little benefit in doing so.  If a custom ICommand implementation has been created (other than RoutedCommands and RelayCommands) then mocking it would be recommended.
Lastly we have a ContactDto property.  Because DTO’s are basically structs I would not bother mocking this out.  This class is simply storing values.

Effectively we can inject our own input into this class and remove its dependencies on all external objects.  When we write tests against FormController Class only code failing inside it can fail the test.



The main goal of Unit Testing is to isolate the subject under test from all other dependencies.  This is to prevent failing dependencies from failing a test that is only trying to see if the subject under test is broken, not its neighbours.

As a rule, enums, scalar types, and consts need not be interfaced.

Consider another example, imagine a Contact class with properties for Surname and FirstName.  During instantiation the class tries to talk to the database to get a requested contact based on constructor arguments.  If the database is offline or not configured for testing then the test will fail, and it is not the Contact class’ fault as such. This could be tested, but only by providing a real test database. In this style of testing there are many moving parts that could fail the test that are outside the immediate subject under test. Imagine testing the Delete method on such a class, data will probably be actually erased from a real database.

To prevent situations like this, dependencies must be “interfaced”.  This means that all outside dependencies the class needs must be typed to an interface not a concrete type.  If database access is done throw an interface, a mock in-memory database can be used instead of a real database, allowing the delete method to be tested, with repeatable consistent results.

Using Inversion of Control (IoC) aka Object Factories

Extensive use of interfaces will require some piece of code somewhere that can tell the application which concrete type to use at runtime.  This can be done in many ways but most common is the use of an object factory, also known as an Inversion of Control container or IoC container for short.  An object factory is essentially a register of what concrete type to use for which interface.

The benefit an object factory gives you is the ability to configure the factory and change its configuration to “test configuration”.  Instead of then manually setting properties using Private Accessors or constructor arguments the class will get its dependencies from the factory. Of course when the application runs in production mode it will be configured to return the real concrete implementations.

Examples of IoC frameworks are Structure Map, Unity, Windsor, Ninject.
Commonly, if you do not make effective use of an object factory you will find your classes have many constructors accepting many different kinds of interface or one constructor with a great deal of arguments.

Object Factory Example

Using Structure Map, getting an instance of an interface is an easy as this:

this.backingMessageBoxService = StructureMap.ObjectFactory.Container.GetInstance<IMessageBoxService>();


As stated previously an object factory is essentially a register mapping interfaces to classes. So to make the above code work, you first need to register the interface IMessageBoxService with the object factory. This can be done in one of two ways; either by code, or from the app.config.
By Code:

ObjectFactory.Initialize(init => { init.For<ILogWriter>().Use<FakeLogWriter>(); });


Or by App.Config:

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
     <configSections>
        <section name="StructureMap" type="StructureMap.Configuration.StructureMapConfigurationSection,StructureMap"/>
     </configSections>
 
    <StructureMap>
         <DefaultInstance
            PluginType="MyAssembly.Namespace.IMessageBoxService,MyAssemblyFileName"
             PluggedType="MyAssembly.Namespace.WpfMessageBox,MyAssemblyFileName"
             />



For more detailed examples on how to use Structure Map please refer to the StructureMap documentation. http://structuremap.github.com/structuremap/index.html

Unit Testing 101 Guidelines Part 2

See also Unit Testing Guidelines Terminology, Part1, Part2, Part3, Part4

What is Integration Testing?

Integration tests test the coupling between much larger components of code compared to unit testing.  While unit testing is intended to test only one member within a class, an integration test is intended to test many classes all at once.  Hence the name “Integration Test”.  Often this means testing the integration between two components like two architectural layers or libraries.

Integration Tests are less frequently used than unit tests. Integration Tests should be stored in a different namespace, they sometimes have more overhead to run them and they may take longer to run.  They can be automated, but because they often require specially constructed environments to run and take time, they are not used in automated builds, gated check-ins or developer testing.  QA Testers would run integration test as part of acceptance testing and regression testing (possibly performance testing also).

A developer will run the unit tests many times through the course of his/her day to test for breakages, and they run quickly.  Integration tests are run less frequently and test far fewer use cases. They are merely intended to test the coupling not every possible or valid permutation of test data input.

For example: In an N-Tier system there may be a presentation layer, a business layer, and a services layer.  A common scenario would be to test some large feature in the business layer and mock out the services layer entirely using a real WCF service but all the services within the service layer are mocked.  This means that most if not all the components in the business layer being tested are real production objects.  Also the WCF communications is actually taking place over real end-points and bindings and actual production messages are being transmitted.  The service layer does host a real WCF service but the services within it are all mocked out to return hardcoded data or the like. In this example all components in the Business layer are being "Integration Tested".

Thursday, December 24, 2009

Unit Testing Terminology

See also Unit Testing Guidelines Part 1, Part 2, Part3, Part4

This is an introduction into a 4 part article on a Beginners Guide to Unit Testing.  This post covers some common terminology used in Unit Testing (and Testing in general).  The aim of the series is to give you a broad introduction into what it is, what the goal of unit testing is, how to do it, and some guidelines on best practice.  The series is based on my experience in the field making unit testing work and ensuring it is an asset to stake holders.  There is also considerable influence on my thinking from numerous blog posts and msdn magazine articles.

Here's a list of unit testing terminology (or terms that can be used in conjunction with unit testing) I use and have heard others use: (This is not a comprehensive list)

  • Accessor.
    See PrivateAccessor.
  • Assertion.
    A process of ensuring a output value from a code component is the expected value it should be. Usually in the form of a statement verifying the output against a (or a list of) known good values. This verifies the statement: "If you control the input you should be able to predict the output".
  • Black Box Testing.
    Testing a software where the tester or unit test author has no knowledge of (or access to) the code they are testing.  This is not generally advisable for unit test authoring.
  • Boundary Values.
    Describe input values to a component that are on the boundary of expected input. Ie, passing in Null or zero, negative values, or values too long or large.
  • Code Coverage.
    A unit of measure as a percent. This measures the percentage of lines of code in an assembly how many have been touched by a unit test. 
  • Combinatorial Test.
    A technique where you specify a list of values for each input parameter and the xUnit test framework will repeatedly execute the test with all possible permutations of the input data into their respective input arguments.
  • Dependency.
    A reference to another class, making the class under examination dependent on the referred class. Replacing dependencies with mocks or stubs is usually referred to as Mocking dependencies. 
  • DTO.
    A Data Transfer Object. An object that only includes properties designed to transfer data across communications channels. This is relevant from a testing point of view because DTO’s are seldom mocked (or stubbed) due to their simple nature. DTO’s are basically STRUCTs (no methods) except they are implemented as classes with properties to comply with coding standards and serialisation restrictions. DTO’s should only use scalar data types, or other DTO types, or interfaces. 
  • Functional Testing.
    Functional tests test a full slice through all architectural layers of a system. They verify a specific business function is performing as it was intended. These are generally not automated.
  • Integration Testing.
    Integration tests are ideally automated and repeatable tests designed to test the interaction of two components with each other in an intended way. Integration tests are usually much larger than unit tests and take longer to run. Integration tests may or may not be automated.
    See a later article for more information. 
  • Mock.
    A more complex form of dependency replacement. Property and Method behaviour can be injected by individual tests. Mocks can be handcrafted but this is generally too much work, they are usually generated by a mocking framework such as Rhino or Moq. Mocks are able to be configured for a particular test and able to report back if all the appropriate members have been accessed and were given the expected input parameters. Thereby testing the output of a class and its interaction with its dependencies. 
  • Pairwise Testing.
    A technique to combat the explosion of possible test permutations using a Combinatorial approach when a large list of values is supplied. This approach works on the assumption that most errors occur with certain combinations of input values, and careful consideration is used to choose these combinations. This approach has not gained a great deal of popularity.
  • PrivateAccessor (or just Accessor).
    A means to set, get and invoke private/protected/internal members of a class. An Accessor usually takes the form of another class with the same name as the target class but with the suffix Accessor. One Accessor is generally coded per class. Accessors are usually not coded manually but are either generated or provided by a Unit Testing framework (such as MsTest or utilities based on reflection). 
  • Regression
    Is a term often used to describe a software component loosing functionality and breaking existing functionality by adding new features. Often used to describe "Regression Prevention Testing" or just "Regression Testing".
  • Stub.
    The simplest form of dependency replacement. Properties simply store a value, methods have no code. The stub does not store any kind of state and nor does it offer any way of reporting if its members have been accessed by the subject under test. A stub is used if the test does not care in any way if the dependency is accessed or used by the test; it is merely “stubbing out” the production dependency class. 
  • Subject Under Test (or SUT).
    This refers to the class that is the target for a particular test. 
  • Test Driven Development (TDD).
    See a later article for more information. 
  • Test Fixture.
    A test class that resides in a test project. A test fixture contains test methods to test a production class. A test fixture targets only one production class. A production class may have many test fixtures that test it. 
  • Unit Testing (UTs).A unit tests are tightly focused automated, repeatable, and independent tests aimed to test a very small piece of code, usually one method. The goal is to test every code path through a method.
    See a later article for more information.
  • xUnit.xUnit refers to the various testing frameworks which were originally ported from sUnit. Tools such as jUnit, qUnit, and nUnit fall into the xUnit family.

This list will grow over time.

References:

Sunday, December 20, 2009

Unit Testing 101 Guidelines Part 1

See also Unit Testing Guidelines Terminology, Part1, Part2, Part3, Part4

What is Unit Testing?

A unit test is a focused automated test for one member in a class.  This means it’s a small piece of .Net code designed to test one member of a class.  Given control of the input to a method the output can be predicted (with knowledge of the code or business function) and then compared to a predetermined constant. If it does not meet this expectation outcome the test fails.  

For example if you have a method Add(x, y) that takes two integers you know (based on the specification) that passing in 2 and 3 should return 5.

Unit tests are built over time and stored in large libraries of tests. These libraries are then used to test a lot of code very quickly and covering a good deal of code.  This fast high code coverage testing can then be used to gauge the success of new code (has a newly added feature somehow broken existing code aka ripple-effect).

The style of inputs generally fall into two categories: common use case input, and fringe input intended to see if the method gracefully handles unexpected data.

Unit tests are intended to be isolated and independent from one another. This means that one test running should not interfere with others also running at the same time or subsequently.  For the sake of speed, applications that run unit tests quite often run tests in parallel on multiple threads; doing so should not fail the tests. Making use of static state variables will cause tests running in parallel to interfere with each other.

Unit testing is strictly targeting development and developers.  However unit tests should always be automated into the master build process.  This gives an indication of build quality and ensures QA testers do not start testing a broken build.  A broken build is further described as a build that does not compile or a build in which not all tests are passing.

Unit testing is only possible on code that has been designed to be tested.  Designing for testing essentially means that class design allows for ways to provide alternatives (stubs and mocks) for all dependencies of the class.

When testing a method within a class you go through a process of “mocking out” dependencies so the class under test does not try and call other production code outside the class. For example to ensure a class does not call a live service or database. This creates an isolated test environment that focuses the testing to the one class and method. Only code in the class under test can fail the test.


What is Test Driven Development (TDD)? 

Test Driven Development is sometimes used in conjunction with Unit Testing. Unit testing can be fully utilised without following a Test Driven Development process. However by following TDD high code coverage is achieved more rapidly and code tends to be more concise and less complex.

WIKIPEDIA: It’s an approach focused on writing only the code necessary to pass tests, resulting in cleaner and clearer designs than is achieved by other methods. It cuts out extra functionality that is outside of the current scope. This definitely results in simpler, easier to implement and follow code, but means that extensibility becomes a problem for future projects unless extensibility is part of the original specification and scope.

The main principles are:

  • —The main principal is to KISS YAGNI. 
  • Keep It Simple Stupid, You Ain’t Gunna Need It. 
  • Developers are given Use Cases with the software specifications. 
  • Write failing tests first that match Use Cases. 
  • Write code only to make tests pass. Results in a clean uncomplicated easy to use application.

Tuesday, December 15, 2009

NZ Home Loans Review - Extremely Poor Service


15-December-2009

I know this is off-topic but I simply must have a rant and warn anyone who will listen about this company and their evident incompetence and lack of interest in righting their mistakes.


I was very shocked disappointed with the results and with the advice I have received.  I was given advice without proper disclosure of conditions, risks, and with disregard for jeopardizing a great deal of my money. My complaint centers around quality of advice and a verbal pre-approval with the single condition that the valuation is to be more than the purchase price at auction.  

I originally approached NZ Home Loans on the 1st of October, and was impressed by their friendliness and informality.  During this first meeting we told our broker that we wished to attend an auction on the 21st of October. We were told it would be easy to turn the application into an approval before then.  After various chasing phone calls I finally received word on the 16th of October, that I had been approved but with a condition that they required a valuation of the property to be completely unconditional.  This was disappointing as there now was not enough time to organise a valuation before the auction on the following Wednesday.  

We discussed this, and I was advised that the valuation is only about ensuring the purchase price is not more than the valuation.  I agreed to take the risk of going to auction with only a verbal conditional pre-approval with the understanding there was only one condition. This condition was simply disclosed to me as obtaining a valuation for the property to ensure the value is more or equal to the purchase price at auction.  The broker arranged a valuer friend to quickly and informally preview the property prior to auction to give some confidence where the value will be. At no time was I made aware that the formal valuation could raise 'other' potential 'declineable' issues. There was no disclosure that despite the valuation price being well clear of our purchase price it could still be declined. In my opinion these were hidden and undisclosed conditions that I was completely unaware of. This is the exact reason why we seek advise from experienced professional people to begin with is it not?

After reading my email trail again, I also cannot help but question the speed of handling our application.  The loan application documents were hand delivered on the 7th of October. The broker was aware of our interest in attending the auction on the 21st of October.  I was notified by telephone on Friday the 16th of pre-approval for my application, but that one condition as stated above. We were originally told prior, that the requirement of a valuation is "unlikely".  So at this point there was not enough time to organise a valuation, or for this valuation to be processed by NZ Home Loans.  So on advice I proceeded with plans to attend the auction knowing that his valuer had previewed the property and was comfortable it was easily well clear of what my personal budget ceiling.  After winning the auction I waited 4 weeks to get this final response. If this was a standard settlement period I would have been forced to default on the settlement. I chased this up during this period with phone calls and emails, usually with no response. One response was "we seem to have lost the documents, can you please resend"!

On Tuesday the 17th I was advised that they had been forced to decline my application!  Then, on the Wednesday I was then told this decision wasn't a 'No' after all, but if we obtain a structural builder's report and moisture report there may be a chance. Failing that, the suggestion was to max out our credit cards to fund an additional 30k deposit!

At that point I decided to ditch NZ Home Loans, and take the risk of finding funding elsewhere. Incidentally, our approval with our new lender was turned around in 36 hours completely unconditional. Big Thanks to Westpac New Zealand. After complaining to NZ Home Loans head office I get a phone call from the manager asking to discuss the situation.  Which I happily did 6 days later and told them exactly how I felt about the whole foul up.  During this meeting, his way of making things right was to offer me an approval for funding; a little late I think to say the least.

I wish I lived in the States where suing these clowns would be much easier. However I haven't ruled out contacting the Ombudsman. At the very least I will be vocal about my experiences to my family, friends, and in my tweets and blog posts. I think there was a severe failure in procedure and it seems to me that competency levels are well short of what they should be.

Please think twice before risking their service. Happy home ownership :-)