Improve Install of targets / crash Install on Databasetarget.

[M4ttsson]

My first PR so I hope I got everything right.
Fixes #1098

[codecov]

Codecov Report

Merging #2103 into master will increase coverage by <1%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #2103    +/-   ##
=======================================
+ Coverage      81%     81%   +<1%     
=======================================
  Files         289     289            
  Lines       19957   19964     +7     
  Branches     2360    2361     +1     
=======================================
+ Hits        16176   16234    +58     
+ Misses       3175    3126    -49     
+ Partials      606     604     -2

[M4ttsson]

Btw, should my unit test run in Travis? I assumed that the other sql test that I "took inspiration" from is not run in Travis for a reason so I used the same if-case to disable it.

[304NotModified]

thanks!

AFAIK Travis doesn't have a SQL service? If it has, it would be great to enable (I'm an other pull request ;)

[304NotModified]

More info is here: https://docs.travis-ci.com/user/database-setup/

[304NotModified]

Thanks! this a great improvement.

But I was thinking, isn't easier to do something like this?

public void Install(InstallationContext installationContext) {
  if(!IsInitialized)
  { 
     InitializeTarget(); 
  }
  ...
}

[M4ttsson]

Yes, it would. I was concerned about any side effects of calling the base.InitializeTarget() that contains calls to a FindAllLayouts() method. I guess it would cause the layouts to be loaded?
I could change it if you want? I agree that the code would look better that way.

[304NotModified]

Indeed the side effect is less clear.

So for now this is good enough, thanks!

[M4ttsson]

Awesome. I think I'll look for another issue to do now. Or should I maybe create an mysql version of the install tests so it can be tested in Travis too?

[304NotModified]

Or should I maybe create an mysql version of the install tests so it can be tested in Travis too?

That would be great also to have!

[304NotModified]

Fixed in 4.4.10 and the release is queued. Will be online in a few hours!

Thanks for the submit!

[M4ttsson]

Awesome.
I might have some time tomorrow to look at creating the mysql install test we mentioned. Should I just do it and create a pull request or how should I do it?

[304NotModified]

Just send a PR. Would be great to have!