search

pupnp / pupnp

libupnp: Build UPnP-compliant control points, devices, and bridges on several operating systems.
https://pupnp.github.io/pupnp
BSD 3-Clause "New" or "Revised" License
353 stars 117 forks source link

Ingo h gtests01a #305

Closed mrjimenez closed 3 years ago

Vollstrecker commented 3 years ago

I still prefer to mark the failing test and run them anyway.

Already done yesterday but was removed from gtest/CMakeLists.txt. Will put it back soon after troubleshooting for tests with mocked functions. That are the only tests that fail but not on all tests. Detailed documentation of disabled tests is done on every test output.

I am having some difficulty in understanding what exactly you are talking about. There are two kinds of test:

  1. Those that have already succeeded and now are part of sanity checking.
  2. Those that have never succeeded and are part of the development of new features or for fixing old wrong functionality.

Tests in the first category are the ones that must be run on Github actions. Tests in the second category must have a way to be run outside github actions.

You can see the test that are not run, because there is a problem in them. It's possible to tell the system that they will fail, so as long as they fail, it's considered passing. If someone fixes this "by accident" these test would succeed and therefore marked as failing, so you can see the problem was fixed. As it could be a sideeffect, noone would try to run the test by hand, so you maybe notice that later, or even to late if "fix" was meanwhile broken again.

It's just not nice to do this with only some tests of the source, and I don't know if it's possible to just put the failing ones in a separate file with too much overhead.

mrjimenez commented 3 years ago

So, are we ok to commit now?

ingo-h commented 3 years ago

Yes, please commit

Vollstrecker commented 3 years ago

If you remove lines 484 and 493 from toplevel CMakeLists.txt, it should be OK. If it fails in Win then (what I expect), you can insert them around the addition in gtest/CMakeLists.txt. Otherwise the tests we already have don't run at all on win.

ingo-h commented 3 years ago

i agree with @Vollstrecker to wrap the tests in gtest/CMakeLists.txt with the if (NOT WIN32) flag and remove it from the toplevel CMakeLists.txt. In the next step I will look at the gtests and try to make them run on MS Windows.