Make tests support empty SPDLOG_EOL

[emmenlau]

This PR is currently a demo only. It is created against tag v1.5.0. It should show-case how one could add support for empty SPDLOG_EOL in the tests. This seems a "relatively" common request amongst users so it may be nice if the tests can support it.

Closes #1413 .

[gabime]

I think it could be cleaner if a new “count_messages()” function was introduced with all the logic in it..

[emmenlau]

I think it could be cleaner if a new “count_messages()” function was introduced with all the logic in it..

Yes, you are right. I'll take a look at it. Do you have a preferred source file where this method would go, since its used in multiple tests?

[gabime]

tests/utils.cpp is the most suitable for this

[emmenlau]

Dear @gabime thanks for the constructive feedback. I've adapted the tests and indeed the code comes out much better. Do you think this is reasonable to add?

[gabime]

sure, but they fail

[emmenlau]

The travis build works except an installation problem with g++7. The Appveyor build is something else, that may be related with newlines on MSVC. That brings up a question. It fails in places where I changed

"Test message 2\n"

to

"Test message 2" + std::string(spdlog::details::os::default_eol)

Its actually interesting how this worked before, because AFAIK on MSVC the SPDLOG_EOL was defined as \r\n. So why was the test expecting only \n? Or am I on the wrong track?

[gabime]

That's because file_contents(filename) open the file in text mode so the \r\n got converted to \n

[emmenlau]

That's because file_contents(filename) open the file in text mode so the \r\n got converted to \n

How should I best solve this? Or can you help? I personally would change the behavior of file_contents() to open in binary mode, but will that change many other tests?

[gabime]

Open in binary mode is better, I agree. I am not sure how it would affect the windows tests, if any. You can try and see

[emmenlau]

Seems I was lucky with this change, the tests now all work? Could you consider this for merging?

[gabime]

Sure, but I would appreciate if you first added the changes I requested.

[gabime]

Thanks @emmenlau . Merged.