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

Fix #8: Update LoggerTests #9

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

NoRePercussions
Copy link

Description of the Change

Use an fmemopen stream rather than a open_memstream stream, as the latter is specified to be write-only.

Also adds new test cases.

Motivation

The previous tests used a technically write-only stream in memory, which causes the tests to fail on some systems. Instead, we can use an fmemopen stream to ensure it is always readable.

The old tests were not comprehensive and may not have detected all issues.

Possible Drawbacks

None anticipated, unless future tests involve writing more than 10,000 characters.

Verification Process

Passes all test cases and Valgrind.

Applicable Issues

#8

The previous tests used a technically write-only stream in memory, which causes the tests to fail on some systems. Instead, we can use an `fmemopen` stream to ensure it is always readable.
If each logger test writes more levels than requested as-is, the tests would not catch it due to the logging order. This way, we can assert that it does not write more.
In its previous form, TestLazyLogging checked that logging didn't error and that the lambda used was scoped. This version will check that it logs the contents given and no more.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant