Add setup instructions for running tests locally. Database required? about conventional HOT 11 CLOSED

from conventional.

All you should require is a default .\SQLEXPRESS instance, the idea being the database convention tests have a .sql file that migrates the db to a success or failure case for each convention, which gets torn down at the end of each test.

That isn't entirely discoverable though - where would you usually expect to see this sort of information - in the wiki with the rest of the doco, or in a seperate CONTRIBUTING.md in the root?

from conventional.

In a CONTRIBUTING.md is standard but it could also be a line at the bottom of the current README.

All you should require is a default .\SQLEXPRESS instance

Ah, I don't have a default .\SQLEXPRESS instance. It wasn't clear from the error messages that was the case. Perhaps we can add check for the instance and throw explicitly if not found? e.g. throw new InvalidOperation("Database tests require a default instance of .\SQLEXPRESS");

from conventional.

Good call @todthomson - probably worth shifting to localDB.

Do you have a good example you can point me at of handling the database file generation for a localDB database for unit testing in .NET Core? That seems to be the pain point after 10 minutes of mucking around / googling - with SQLEXPRESS you just point and use, with localdb you need to ensure it generates the database files somewhere sensible, and potentially clean them up after too.

from conventional.

Looking into it now, I'll report back my success/failure.

from conventional.

All is looking good with my installation of Microsoft SQL Server 2016 LocalDB (which was installed with Visual Studio 2017, I didn't install it separately).

All I did was update DatabaseConventionSpecificationTests.cs replacing .\SQLEXPRESS with (localdb)\MSSQLLocalDB and then "Run All Tests from Solution".

Conventional.mdf and Conventional_log.ldf get created in C:\Users\%username% and then successfully removed at the end of test run i.e. I can see them get created/deleted and I can run the tests (all passing) multiple times in a row.

@andrewabest Let me know if you would like a PR?

from conventional.

I'm also wondering if the following:

#if DEBUG
        private const string TestDbConnectionString = @"Server=.\SQLEXPRESS;Database=Conventional;Integrated Security=true;";
#else
        private const string TestDbConnectionString = @"Server=(local)\SQL2014;Database=Conventional;User ID=sa;Password=Password12!";
#endif

could simply be replaced with just:

private const string TestDbConnectionString = @"Server=(localdb)\MSSQLLocalDB;Database=Conventional;Integrated Security=true;";

i.e. I'm not sure why the DEBUG vs RELEASE runs against different databases, but there could well be a good reason for this...

from conventional.

@todthomson I ran smack bang into https://dba.stackexchange.com/a/191748 when attempting the same change, which means other people are likely to as well.

Which makes me wonder if switching over is a good thing if we need to caveat it with "If you hit this bug you need to update to SQL 2017 CU 6" 😭

I'm pondering a try/catch around CreateDatabase() in Setup() to trap the SQL exception and suggest that they may need to update if they hit it. Thoughts?

Also - the reason DEBUG and RELEASE have different dbs for tests at the moment is just CI infrastructure. If the agents have localdb, we can bring these back to a single statement, which would be #winning.

from conventional.

Also - the reason DEBUG and RELEASE have different dbs for tests at the moment is just CI infrastructure. If the agents have localdb, we can bring these back to a single statement, which would be #winning.

No localdb on AppVeyor, only SQL Express https://www.appveyor.com/docs/services-databases/ 😞

from conventional.

I'm pondering a try/catch around CreateDatabase() in Setup() to trap the SQL exception.

Given that users may need to upgrade their SQL to 2017 CU6 (which I would be reluctant too as want to keep my local SQL Server same as production), it may be best to stick with SQLEXPRESS and add the try/catch to say that you need to have a local instance.

from conventional.

The other option would be to support a development.json file in the solution root which would contain a database connection string. It would be excluded via gitignore, but if it existed, we would use the connection string from there. Could have a development.json.example template that could be followed.

This allows you to BYO DB if you don't have SQL express, but doesn't end up with you having to revert code changes when you are done.

That combined with the try/catch and some helpful hints as to what the problem may be could do.

Thoughts @todthomson @dennisroche?

from conventional.