search

getml / reflect-cpp

A C++20 library for fast serialization, deserialization and validation using reflection. Supports JSON, BSON, CBOR, flexbuffers, msgpack, TOML, XML, YAML / msgpack.org[C++20]
https://getml.github.io/reflect-cpp/
MIT License
901 stars 76 forks source link

Use Google Test or similar testing framework #34

Closed liuzicheng1987 closed 4 months ago

liuzicheng1987 commented 8 months ago

Pertaining to the discussion related to @zerolfx's great PR (https://github.com/getml/reflect-cpp/pull/29), it is necessary to use some kind of testing framework to fully reap the benefits of Github Actions. We are looking to introduce more serialization formats over the next 2-3 weeks. Being able to test them all in an automated manner will be a great benefit that I really look forward to.

ChemiCalChems commented 8 months ago

For starters, we don't really need to move to Google Test right away. Making sure our current tests return true or false so the test process can return 0 on success and non-zero on failure is enough to get the GitHub Actions saga rolling.

I'm not saying I'm against introducing Google Test (in fact I was quite surprised when I first looked at the project and I saw no test library at all being used), I'm just saying as it stands, a quick PR changing a few things in our tests is enough for GitHub Actions for the time being.

liuzicheng1987 commented 8 months ago

@ChemiCalChems , sure, returning 0 or non-zero would work as well.

To be fair, checking whether the tests compile is more important than making sure they run through. Usually, when the compile, they also run through. And we can do that with Github Actions without changing anything at all.

ChemiCalChems commented 8 months ago

The changes to be done in the tests without having to introduce Google Test are so minimal I'd go for it in one go. I'll be free to write some code in the latter half of the week so if no one has made the changes by then, I'll do it so we don't have @zerolfx waiting around on that for his PR to be merged.

ChemiCalChems commented 8 months ago

Currently working on this issue. Have already made some progress, will try to submit a pull request tomorrow.