Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve mocking of file target (current date) #1022

Closed
304NotModified opened this issue Nov 8, 2015 · 3 comments
Closed

Improve mocking of file target (current date) #1022

304NotModified opened this issue Nov 8, 2015 · 3 comments
Labels
enhancement Improvement on existing feature file-target up-for-grabs
Milestone
4.3

Comments

@304NotModified
Copy link
Member

There are/were some bugs with file archiving which are difficult to (unit) test. Most of the time this has to do with the usage of current date(times) .

Proposal : inject "current" date into file target for better and more robust testing.

Related: #859, #888, #964.
@304NotModified 304NotModified added up-for-grabs enhancement Improvement on existing feature labels Nov 8, 2015
@304NotModified 304NotModified mentioned this issue Nov 11, 2015
Closed
@304NotModified 304NotModified changed the title Improve mocking of file target Improve mocking of file target (current date) Nov 11, 2015
@304NotModified
Copy link
Member Author

Maybe use timesource? It's already (misused) for current date in the filetarget

@bhaeussermann
Copy link
Contributor

I'm looking into this. Some of the mentioned issues above are not yet resolved, so some tests are going to fail. I shall have these tests Skipped for now. Will that be in order?

@304NotModified
Copy link
Member Author

Great!

Those issues are indeed not (all) fixed.

I was thinking :

  • make mockable, pr and merge
  • for every issue, create unit test, fix if needed, and that in one PR. (TTD)

It's okay to add unit tests with ignore if you have them already.

bhaeussermann added a commit to bhaeussermann/NLog that referenced this issue Dec 19, 2015
@bhaeussermann
FileTarget: Fix archive based on time does not always archive.
3a76075 
See issues NLog#1022, NLog#859, NLog#550
@bhaeussermann bhaeussermann mentioned this issue Dec 19, 2015
Merged
@304NotModified 304NotModified added this to the 4.3 milestone Jan 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement Improvement on existing feature file-target up-for-grabs
Projects
None yet
Milestone
4.3
Development

No branches or pull requests
2 participants
@304NotModified @bhaeussermann