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.

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

DatabaseTarget should also dispose connection on close #610

Merged
merged 3 commits into from
Feb 27, 2015
Merged

DatabaseTarget should also dispose connection on close #610

merged 3 commits into from
Feb 27, 2015

Conversation

dodexahedron
Copy link
Contributor

Fixed CloseConnection() leaks IDbConnection objects as reported in #609

Fixed CloseConnection() leaks IDbConnection objects as reported in #609
@304NotModified
Copy link
Member

The unit tests won't expect the dispose . Can you fix that?

Thanks!

@dodexahedron
Copy link
Contributor Author

I'll have a look.

@304NotModified
Copy link
Member

FYI: It's not a difficult issue, some tests at NLog.UnitTests.Targets.DatabaseTargetTests are failing, because they read the log.

For example:

NLog.UnitTests.Targets.DatabaseTargetTests.KeepConnectionOpenBatchedTest2 [FAIL]
   Assert.Equal() Failure
   Position: First difference is at position 188
   Expected: Open('Database=MyLogger').
ExecuteNonQuery: INSERT INTO FooBar VALUES('msg1')
ExecuteNonQuery: INSERT INTO FooBar VALUES('msg2')
ExecuteNonQuery: INSERT INTO FooBar VALUES('msg4')
Close()
Open('Database=MyLogger2').
ExecuteNonQuery: INSERT INTO FooBar VALUES('msg3')

   Actual:   Open('Database=MyLogger').
ExecuteNonQuery: INSERT INTO FooBar VALUES('msg1')
ExecuteNonQuery: INSERT INTO FooBar VALUES('msg2')
ExecuteNonQuery: INSERT INTO FooBar VALUES('msg4')
Close()
Dispose() <!-------------------------------------------------------------------------- NEW 
Open('Database=MyLogger2').
ExecuteNonQuery: INSERT INTO FooBar VALUES('msg3')

@dodexahedron
Copy link
Contributor Author

All unit tests in that class pass, now.

@304NotModified 304NotModified added the enhancement Improvement on existing feature label Feb 27, 2015
@304NotModified
Copy link
Member

Thanks!

@304NotModified 304NotModified added this to the 4.0 milestone Feb 27, 2015
304NotModified added a commit that referenced this pull request Feb 27, 2015
@304NotModified 304NotModified merged commit 1da197f into NLog:master Feb 27, 2015
@dodexahedron dodexahedron deleted the patch-1 branch February 28, 2015 06:37
@snakefoot snakefoot changed the title Fixed NLog/NLog#609 DatabaseTarget should also dispose connection on close Dec 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database-target enhancement Improvement on existing feature size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants