Issue #987: Filetarget: Max archives settings sometimes removes to many files

[bhaeussermann]

Fixes #987 basically by fixing the order of the actions:

• Do auto-archive before deleting old archives (this way when there's a new archive, this new archive is accounted for when deleting old archives)
• During archive: Call RollArchiveForward() before EnsureArchiveCount()

[304NotModified]

Thanks!

Any idea why appveyor complains?

[bhaeussermann]

So ThreadSafe_getCurrentClassLogger_test fails. That's strange. The test passes on my side and I didn't touch LogManager.

[304NotModified]

I will rerun the build. Some tests aren't robust, but I haven't seen failing this test before.

Note: I also tried to remove the (directorynotfound) try-catch before, but then also some tests failed.

Some really good renames and small improvements in code, I'm impressed!

[codecov-io]

Current coverage is 71.86%

Merging #1019 into master will decrease coverage by -0.09% as of f20288e

@@            master   #1019   diff @@
======================================
  Files          263     263       
  Stmts        14829   14827     -2
  Branches      1610    1612     +2
  Methods          0       0       
======================================
- Hit          10670   10655    -15
+ Partial        420     418     -2
- Missed        3739    3754    +15

Review entire Coverage Diff as of f20288e

---

Uncovered Suggestions

1. +0.08% via ...argets/FileTarget.cs#1918...1930
2. +0.08% via ...argets/FileTarget.cs#1555...1567
3. +0.08% via ...c/NLog/LogFactory.cs#180...191
4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

[304NotModified]

This seems another issue, I fix that one in another PR.

[304NotModified]

We lost coverage by removing an unit test?

[bhaeussermann]

We lost coverage by removing an unit test?

I changed the MaxArchiveFilesWithDate() test helper method to always do an exact compare instead of lower-or-equals by removing the exactArchiveTest parameter.
After this change, the TestMaxArchiveFilesWithDate test had made exactly the same call to MaxArchiveFilesWithDate() than MaxArchiveFilesWithDate_removesToManyFiles, making the latter test superfluous. Hence I removed that test. Both tests invoked the same code, so the code coverage should not be affected by this.

Judging by the code coverage diff, it appears that all added code is covered by the tests.

I would attribute the drop in coverage percentage to the fact that the number of statements in FileTarget has decreased (58 additions & 62 deletions in the main commit). What do you think?

[304NotModified]

I think your right! Thanks for checking!