Make gtests running with new user interface

[ingo-h]

Improved the gtests and make them running with the new user interface with the UpnpLib structure.

[Vollstrecker]

Could you maybe just uncomment lines 380-387 from the main CMakeLists.txt? I mean, it's nice to have a shell-script with hardcoded paths to thirdparty-software which just isn't there to have it run in the automated test-env, but I would like more to see if it works. With this you could also see if points 2 and 3 of your ToDo is already done.

Or do I get that wrong, and this is our task, and if you extend this to work on win, we'll get some VS-projects for this. Btw. if it doesn't work on win, uncomment the lines 379 and 388 also to deactivate on win.

And yes, if something cmake-related goes wrong, you can expect me to send you a pr with the fixes, so it will be merged with your changesm which I consíder the more correct way than doing that afterwards.

[mrjimenez]

Hi Ingo,

I am a bit confused on how am I supposed to use your infrastructure to test.

I did

$ ../compile.sh test_tools.cpp 
g++: error: /home/user/devel/pupnp-dev/pupnp/lib/libgtestd.a: No such file or directory
g++: error: /home/user/devel/pupnp-dev/pupnp/lib/libgmockd.a: No such file or directory
g++: error: /home/user/devel/pupnp-dev/pupnp/upnp/libupnp.a: No such file or directory
g++: error: /home/user/devel/pupnp-dev/pupnp/ixml/libixml.a: No such file or directory

Do I have to install something by hand on my system?

If that is work in progress, I could commit it since it does not currently affects the general tests, but maybe you should consider Vollstrecker 's suggestions.

BTW, @Vollstrecker , I am also a bit confused about the line numbers you are mentioning, they don't make much sense when I look at CMakeLists.txt. Lines 379 and 388 are not commented here, but maybe I am looking at the wrong file.

[Vollstrecker]

https://github.com/pupnp/pupnp/blob/d1247f083cf01f1774615c020e80cb6233c4d6eb/CMakeLists.txt#L480

And for the compile-errors, that's what I meant. He once let cmake download (and maybe build) gtest and then hardcoded all paths to there. This was before the change with the libname. The "all produced bin's got to ./lib/" was the workaround for that to let the tests pass.

[ingo-h]

@mrjimenez Googletest and Googlemock isn't downloaded and installed anymore with cmake as before. That is of course needed. I will look after the suggestions as far as we have a stable cmake environment that can run the gtests without configure and builds on it and accept pull requests.

[Vollstrecker]

What do you mean with "without configure and builds on it"?

[ingo-h]

By the way, as evaluated from the README https://github.com/pupnp/pupnp#107-cmake-build I have used before

cmake . -DDOWNLOAD_AND_BUILD_DEPS=ON -DBUILD_TESTING=ON
cmake --build .

There is a typo in the README that will give an error message. There is a missing S at then end in -DDOWNLOAD_AND_BUILD_DEP=ON. Also chapter 5.2 for other options isn't available.

[Vollstrecker]

Seems like one test fails, but at least they compile. Win is activated, but (I guess) the fork was done before the fixes for tv_device, so I can't see if this would work, but deactivation is already prepared in there.

[Vollstrecker]

Ups, wrong button

[ingo-h]

@mrjimenez To elaborate to your questions in more depth:

Do I have to install something by hand on my system?

The compile.sh is just for a quick test and not the final solution. You can edit it and point the BUILD_DIR= to even that. But it will not find libgtestd.a and libgmockd.a if it isn't installed from an old installation. From a initial pull it will not install as before with:

cmake . -DDOWNLOAD_AND_BUILD_DEPS=ON -DBUILD_TESTING=ON
cmake --build .

If that is work in progress, I could commit it since it does not currently affects the general tests

Yes, it is work in progress and the compile.sh cannot be used for production. But to have the gtests available a commit would be helpful.

maybe you should consider Vollstrecker 's suggestions.

Of course, but it is difficult for me to check it without Googletest and Googlemock installed.

@Vollstrecker can you please provide download and installation of Googletest and Googlemock again as already done before? But please don't modify things in the gtest directory at this time. I need the current situation to verify what's going on also with your suggestions.

[Vollstrecker]

https://github.com/ingo-h/pupnp/pull/1

[ingo-h]

@Vollstrecker merged the patch and Googletest and Googlemock is installing now again. But we have also the same situation as before!

The gtests itself MUST NOT run on pull request. It does not make sense. We have tests that WILL fail because of found bugs in the library https://github.com/pupnp/pupnp/issues/247 https://github.com/pupnp/pupnp/issues/272. The tests are made to show it to the responsible developer and he can check his fix against the successful running test then. That is the workflow. Another issue is that I cannot ensure at time that the tests can run successful on the pull request check because i don't know the test environment there. Of course unit tests should run isolated independent from the environment but at this stage they are still too "weak" and does not catch all side effects.

So what is to do to switch off running gtests on pull request?

[Vollstrecker]

I don't think you have the right concept of tests. Test are to be run on pull-requests to ensure that a change doesn't break anything, If you want to deliver a proof of a failure, split it out to a separate file that can be built as a separate bin.

Or better change the test that it succeeds as long as the error is present and name it so everyone can see that something is wrong when this succeeds.

If it's the workflow like you describe it, it's just wrong. What project takes the time to implement gtest with gmock, just to demonstrate something is not working. You described the scenario at least good enough that I understand the problem, so I'm pretty sure the others got a good idea of where the problem is, and they will sort it out.

And for the defined test-env. You insisted that every test should be mockable. The concept of mocking is to define my own env independendly from the physical present one.

[Vollstrecker]

And yes, failing test should reside in a separate file, because when @mrjimenez got the problem fixed, he can just throw away that one, and the one that should succeed if the problem is fixed, then just needs to be activated to make sure it won't be broken again.

[Vollstrecker]

As you tend to not see it: https://github.com/ingo-h/pupnp/pull/2

I marked 4 tests as WILL_FAIL. The init-test fails on ubuntu 18 and 20 while succeeding on ubuntu 16. I'll leave it up to you to interpret the output.

[ingo-h]

@Vollstrecker

I don't think you have the right concept of tests. [..]
If it's the workflow like you describe it, it's just wrong.

No.

This is not the place to explain how Test-driven development works but you should make you familiar with it as described at Wikipedia:

Test-driven development (TDD) is a software development process relying on software requirements being converted to test cases before software is fully developed, and tracking all software development by repeatedly testing the software against all test cases. This is opposed to software being developed first and test cases created later.

[Vollstrecker]

and tracking all software development by repeatedly testing the software against all test cases

There it is. When you want repeatedly testing, you can either rely on each dev running the tests before they commit (even the ones that po up, fixing (or trying to) something and then never come back), or you can automated testing and test everything publicly before comitting anything.

[ingo-h]

I want to do my work within the gtest directory without disturbing other people work and without always fighting against your restrictions.

[Vollstrecker]

Why my restrictions.marcello wasn't even able to compile your work, I provided you the framework because you just said you want to migrate the tests.

If you want to do your work, then you should first clarify what this is. You want to write tests, test that are needed to be compiled by hand, and this just to prrof you've found a problem. Are you going to fix it then, or should someone else do this and then delete the "tests", or adjust them to reflect the fix?

You asked how gtest can be integrated, so I stalled 2 other projects to integrate this, and to integrate everything that comes with it into CmDaB (was planned for later anyways but didn't fit the shedule). 3 days of work later this was up and running. Then you came up with "how should this work", some explanations later I again stalled the other works and made it easier for you again. Now you come with "it's not integrated" and "it's not supposed to be integrated" and "It's just my stuff, integrate it so I can use it and then ignore it".

So yes, my rule is just "tell me what you want or I'll just do nothing and ignore you". If you want tests that fail, you can, but don't expect me to do anything about it. I just wait until the maintainer makes a decision, maybe tell him if I have concerns (and he is someone that isn't pissed to explain it's reasons and is able to name what he wants), and then I'll do exactly what's the outcome of this.

[mrjimenez]

Hi Folks,

Please correct me if I am wrong.

Since as Ingo stated that this is work in progress, I have nothing against it as long as it does not breaks the sanity checks, which it currently is not the case. So fix it and I will commit.

Regarding compile.sh, again, since this is WIP, no problems here, but I would also like to run the code, so I ask you to leave the instructions in a comment in this file, something like "Change this variable according to your path", "don't forget to run $ cmake bla bla bla" or "you must install lib_blablabla from your distribution", so that I know what I am missing.

I understand that all this will eventually become part of the sanity checks, but for now it must be enabled just enough to not make the checks fail, or am I missing something?

Other than that, I thing this is good work and certainly worth developing.

[Vollstrecker]

The wip-status itself is no problem at all.

Since as Ingo stated that this is work in progress, I have nothing against it as long as it does not breaks the sanity checks, which it currently is not the case.

As you can read, the original plan was that the tests fail until #247 #272 are fixed. So until then you'll get always failing tests and would need to check if it's only that one, or maybe more. That's why I want to mark the test failing, so you just have to remember that these problems are unfixed as long as you get no error, but you are able to see one look if something broke. -> https://github.com/ingo-h/pupnp/pull/2

Unfortunately the tests are not that reliable atm -> https://github.com/Vollstrecker/pupnp/actions/runs/692231605 If they or OK on win is hard to guess, at it doesn't get this far, but deactivation is prepared.

but I would also like to run the code, so I ask you to leave the instructions in a comment in this file,

mkdir _deps cd _deps git clone https://github.com/google/googletest.git googletest-src mkdir googletest-build cmake ../googletest-src make cd ../.. mkdir libs cp libgtestd.a libgmockd.a lib/ BUILD_DIR= compile.sh You could adjust the output of googletest by setting their BINARY_DIR to the right place, or you could install it with DEST_DIR and change the path's in comile.sh according, if you (like me) don't like compile-artifacts in your source, you could adjust the whole script, but for now this is the way to go.

[ingo-h]

If you want to do your work, then you should first clarify what this is. You want to write tests, test that are needed to be compiled by hand, and this just to prrof you've found a problem. Are you going to fix it then, or should someone else do this and then delete the "tests", or adjust them to reflect the fix?

Sorry, I wasn't aware that the principles of test driven development are not known. So my in several comments written requirements may be confusing if not seeing this context. I will write a summary and job specification under Discussions so we can discuss it in context. But please give me some hours.

[ingo-h]

https://github.com/ingo-h/pupnp/pull/2 Fixes for failing tests are still execute failing tests and prevent pull request.

[Vollstrecker]

Sorry, I wasn't aware that the principles of test driven development are not known.

That's not true, and you know that. Automated tests are the heart of ttd, and fact that this automation is not wanted, is my problem. You can look at this here, everyone can see the tests are failing, marcello could if all succeed just merge without much handwork, even if he's short on time. That's the benefit. Noone wants to clone the repo (yours in that case) compile some tests by hand to see what happens and maybe copy output back here to discuss it.

Fixes for failing tests are still execute failing tests and prevent pull request.

Yep, but they work on one system and fail on another, so there's a problem within the test itself. I'm not sure why, but you can see that it's not the same everytime. Tests should be able to give the same result, at least on such related system like 16.04. and 18.04. of Ubuntu.

[Vollstrecker]

But to not just criticise, this matures to a good point. I've set just 2 to fail in the end (called_with_invalid_interface), as this was the way I got a successful run at my machine. If I was wrong with that, I think you should be able to transfer the scheme to others one. And noone cares about the win-fails, as this is not related to the tests or anything you've touched.

But the fails are showing someting:

On Debian Testing and Ubuntu 16.04 in Release-Build and 18.04 in Debug-Build this works. So the basic thing works.

On the remaining 4 UpnpApiIPv4TestSuite.initialize_default_UpnpInit2

• UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_valid_interface fails 1x
• UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_valid_interface-static fails 2x
• UpnpApiIPv4TestSuite.initialize_default_UpnpInit2-static fails 2x
• UpnpApiIPv4TestSuite.initialize_default_UpnpInit2 fails 1x

As this happens in different combinations, and sometimes the static link works while the dynamic one fails and vice versa, my guess would be some sort of race-condition. If this is within the test: Sorry, I can't help you there, but you still get some finetuning to do. But maybe this rc comes from the lib or the new interface, then I have to say: Nice catch.

[mrjimenez]

Hi,

Let me add that there is a good chance that my last changes have introduced bug, as all new code does. So definitely testing is something important right now.

Ingo, if we have something mature now, I will try to understand your code and see why it is failing because you might be onto something important here. Just understand that I can't commit it unless the tests are working. If they are not working, maybe we need to fix the library at the same time.

Regards, Marcelo.

[mrjimenez]

    UpnpLib* p = UpnpLib_new();

    EXPECT_STREQ(UpnpGetErrorMessage(
                  UpnpGetIfInfo(p, "if0v4")),
                  "UPNP_E_SUCCESS");

This is not supposed to work. Let me explain why.

The only currently supported way to use libupnp is by calling UpnpInit2() as the first thing. As a consequence, the code inside UpnpInit2() is more constructor code than anything else. In spite of its deceptive name, UpnpLib_new() is automatically generated and does only basic member initialization, mostly setting stuff to zero or other exceptional value. Functional initialization is nonexistent and this is performed inside UpnpInit2().

Tthe first thing UpnpInit2() does is to call UpnpSetLogFileNames() to set up the log file. UpnpGetIfInfo(), as any other function inside the library can only be called after UpnpInit2() because it assumes all the constructor stuff has already happened. I might be possible to get away with not calling UpnpInit2(), but that requires analyzing both UpnpGetIfInfo() and UpnpInit2() to make sure UpnpGetIfInfo() will not use some uninitialized resource.

Maybe this can explain other failures too.

[mrjimenez]

Hi Ingo,

Another interesting thing:

15/22 Testing: test-upnp-UpnpApiIPv4TestSuite.initialize_default_UpnpInit2
15/22 Test: test-upnp-UpnpApiIPv4TestSuite.initialize_default_UpnpInit2
Command: "/home/user/programs/pupnp/maint/github-creator/build/gtest/test-upnp-upnpapi" "--gtest_filter=UpnpApiIPv4TestSuite.initialize_default_UpnpInit2"
Directory: /home/user/programs/pupnp/maint/github-creator/build/gtest
"test-upnp-UpnpApiIPv4TestSuite.initialize_default_UpnpInit2" start time: Mar 28 22:33 -03
Output:
----------------------------------------------------------
Note: Google Test filter = UpnpApiIPv4TestSuite.initialize_default_UpnpInit2
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from UpnpApiIPv4TestSuite
[ RUN      ] UpnpApiIPv4TestSuite.initialize_default_UpnpInit2
2021-03-28 22:33:46 UPNP-API_-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/api/upnpapi.cpp:260]: Inside UpnpInitPreamble
2021-03-28 22:33:46 UPNP-API_-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/api/upnpapi.cpp:277]: Trying a write lock
2021-03-28 22:33:46 UPNP-API_-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/api/upnpapi.cpp:277]: Write lock acquired
2021-03-28 22:33:46 UPNP-API_-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/api/upnpapi.cpp:279]: Trying Unlock
2021-03-28 22:33:46 UPNP-API_-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/api/upnpapi.cpp:279]: Unlocked rwlock
2021-03-28 22:33:46 UPNP-API_-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/api/upnpapi.cpp:417]: UpnpInit2 with IfName=NULL, DestPort=0.
2021-03-28 22:33:46 UPNP-API_-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/api/upnpapi.cpp:4073]: Interface name=if0v4, index=0, v4=192.168.99.3, v6=, ULA or GUA v6=
2021-03-28 22:33:46 UPNP-API_-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/api/upnpapi.cpp:333]: Entering UpnpInitStartServers
2021-03-28 22:33:46 UPNP-MSER-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/miniserver/miniserver.c:936]: get_miniserver_sockets: resuseaddr is set.
2021-03-28 22:33:46 UPNP-MSER-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/miniserver/miniserver.c:598]: sockfd = 6, .... port = 0
2021-03-28 22:33:46 UPNP-MSER-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/miniserver/miniserver.c:598]: sockfd = 7, .... port = 0
2021-03-28 22:33:46 UPNP-MSER-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/miniserver/miniserver.c:598]: sockfd = 8, .... port = 0
2021-03-28 22:33:46 UPNP-MSER-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/miniserver/miniserver.c:961]: get_miniserver_sockets: bind successful
2021-03-28 22:33:46 UPNP-MSER-2: Thread:0x0000000000002ED1 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/miniserver/miniserver.c:598]: sockfd = 9, .... port = 0
2021-03-28 22:33:46 UPNP-MSER-2: Thread:0x0000000000002ED7 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/miniserver/miniserver.c:264]: miniserver 0: READING
2021-03-28 22:33:46 UPNP-MSER-2: Thread:0x0000000000002ED9 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/miniserver/miniserver.c:264]: miniserver 0: READING
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED7 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:457]: (http_RecvMessage): Error -207, http_error_code = 400.
2021-03-28 22:33:46 UPNP-MSER-2: Thread:0x0000000000002EDA [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/miniserver/miniserver.c:264]: miniserver 0: READING
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED9 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:457]: (http_RecvMessage): Error -207, http_error_code = 400.
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED9 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : HTTP/0.0 400 
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002EDA [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:457]: (http_RecvMessage): Error -207, http_error_code = 400.
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002EDA [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : HTTP/0.0 400 
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002EDA [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : Bad Request
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED7 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : HTTP/0.0 400 
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED7 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : Bad Request
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED7 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : SERVER: 
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED7 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : Linux/5.11.6-1-default, UPnP/1.0, Portable SDK for UPnP devices/1.16.0

2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED7 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : CONTENT-LENGTH: 
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED7 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : Accept-Ranges: bytes
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED7 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : CONTENT-TYPE: 
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED7 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : text/html
2021-03-28 22:33:46 UPNP-HTTP-3: Thread:0x0000000000002ED7 [/home/user/programs/pupnp/maint/github-creator/upnp/src/genlib/net/http/httpreadwrite.c:1769]: Adding a string : <html><body><h1>400 Bad Request</h1></body></html>
...

That looks very promising! See HTTP/0.0? That might be something to look into. Unfortunately the ports are returning zero, that might leave parts of the library upset.

But the miniserver is responding and the network mocking infrastructure seems to be progressing, and that is really good news!

Unfortunately, I don't get the errors on the sanity checks, but you may try to fix things on the lines of my previous message.

Regards, Marcelo.

[mrjimenez]

In fact, your tests have already given this result #279 .

[ingo-h]

Hi @mrjimenez,

    UpnpLib* p = UpnpLib_new();

    EXPECT_STREQ(UpnpGetErrorMessage(
                  UpnpGetIfInfo(p, "if0v4")),
                  "UPNP_E_SUCCESS");

This is not supposed to work. Let me explain why.

UpnpGetIfInfo(), as any other function inside the library can only be called after UpnpInit2()

No. I have inspected the code and do the same initialization to UpnpGetIfInfo() that UpnpInit2() do. The upnp/gtest/test_upnpapi.cpp is running successful on this test. See below.

I might be possible to get away with not calling UpnpInit2(), but that requires analyzing both UpnpGetIfInfo() and UpnpInit2() to make sure UpnpGetIfInfo() will not use some uninitialized resource.

What should it be? At the beginning of making the test it fails of course. But then I found all side effects step by step and mocked them. There are no uninitialized resources anymore. The test is running now as shown:

gtest$ ./compile.sh test_upnpapi.cpp && ./test_upnpapi.a 2>/dev/null
[==========] Running 5 tests from 2 test suites.
[----------] Global test environment set-up.
[----------] 3 tests from UpnpApiIPv4TestSuite
[ RUN      ] UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_interface_if0v4
[       OK ] UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_interface_if0v4 (0 ms)
[ RUN      ] UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_unknown_interface
test_upnpapi.cpp:216: Failure
Expected equality of these values:
  *UpnpLib_get_gIF_NAME(p)
    Which is: { 'e' (101, 0x65), 't' (116, 0x74), 'h' (104, 0x68), 'O' (79, 0x4F) }
  *&""
    Which is: 0x55af56cbf1bc
ATTENTION! There is a wrong upper case 'O', not zero in "ethO"

[  FAILED  ] UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_unknown_interface (0 ms)
[ RUN      ] UpnpApiIPv4TestSuite.initialize_default_UpnpInit2
test_upnpapi.cpp:273: Failure
Value of: captFdObj.print(std::cerr)
  Actual: true
Expected: false
Output to stderr is true. There should not be any output to stderr.

[  FAILED  ] UpnpApiIPv4TestSuite.initialize_default_UpnpInit2 (62 ms)
[----------] 3 tests from UpnpApiIPv4TestSuite (66 ms total)

[----------] 2 tests from UpnpApiTestSuite
[ RUN      ] UpnpApiTestSuite.get_handle_info
[       OK ] UpnpApiTestSuite.get_handle_info (0 ms)
[ RUN      ] UpnpApiTestSuite.get_error_message
[       OK ] UpnpApiTestSuite.get_error_message (0 ms)
[----------] 2 tests from UpnpApiTestSuite (0 ms total)

[----------] Global test environment tear-down
[==========] 5 tests from 2 test suites ran. (70 ms total)
[  PASSED  ] 3 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_unknown_interface
[  FAILED  ] UpnpApiIPv4TestSuite.initialize_default_UpnpInit2

 2 FAILED TESTS

Test with interface name if0v4 is OK:

[ RUN      ] UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_interface_if0v4
[       OK ] UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_interface_if0v4 (0 ms)

This is issue https://github.com/pupnp/pupnp/issues/247 and FAILED:

ATTENTION! There is a wrong upper case 'O', not zero in "ethO"

[  FAILED  ] UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_unknown_interface (0 ms)

This is issue https://github.com/pupnp/pupnp/issues/272 and FAILED also:

Output to stderr is true. There should not be any output to stderr.

[  FAILED  ] UpnpApiIPv4TestSuite.initialize_default_UpnpInit2 (62 ms)

[mrjimenez]

Hi Ingo,

Maybe I am being too naive, but I still don't understand how this code works, so please do explain it to me.

...
    UpnpLib* p = UpnpLib_new();

    EXPECT_STREQ(UpnpGetErrorMessage(
                  UpnpGetIfInfo(p, "if0v4")),
                  "UPNP_E_SUCCESS");
...

This is C++. There is no doubt that the first statement executes before whatever is the second statement, which is a macro, btw. So, yes, maybe these google test macros can invert the order of the execution of stuff. But that is not the case, because if I do:

...
    EXPECT_STREQ(UpnpGetErrorMessage(
                  UpnpGetIfInfo(p, "if0v4")),
                  "UPNP_E_SUCCESS");
    UpnpLib* p = UpnpLib_new();
...

I.e., if I invert the order of the statements, I get: test_upnpapi.cpp:211:33: error: use of undeclared identifier 'p'. What you are telling me is that between the execution of UpnpLib* p = UpnpLib_new(); and the macro EXPECT_STREQ there is some code that as you state "But then I found all side effects step by step and mocked them. There are no uninitialized resources anymore.".

Please explain me where (in what lines) you do initialize the contents of the struct pointed to by p.

If you don't (manually?) initialize this struct, I shall stand by my claim that what you get is a fully uninitialized struct that will likely segfault. Notice the use of likely in the previous sentence, because what really happens is undefined behavior, and undefined behavior sometimes runs smoothly.

Please forgive my lack of knowledge of the google test infra-structure, it is certainly in my TODO list to start using it as it already shows very interesting results.

[ingo-h]

Hi @mrjimenez,

I will try step by step. Googletest provides macros to check assertions and expectations as documented at Googletest - primer.md. I had a look at UpnpInit2() where it calls UpnpGetIfInfo(). No problem to find it:

p = UpnpLib_new();
--- snip ---
/* Retrieve interface information (Addresses, index, etc). */
retVal = UpnpGetIfInfo(p, IfName);
if (retVal != UPNP_E_SUCCESS) {
        goto exit_function;
}

So I expect that it also will successfully run under isolated test conditions. This mode is called UUT (Unit under Test). Just copied needed things as shown in UpnpInit2()

//EXPECT_EQ(val1, val2);
p = UpnpLib_new();
EXPECT_EQ( UpnpGetIfInfo(p, "if0v4"), 0 );

This is the situation we do not know anything about the needed conditions and suspect that it can only run if called by UpnpInit2(). And of course this expectation failed, claiming that UpnpGetIfInfo() does not return 0 but instead another number, maybe -127 (can't remember). It was annoying to always look at the translation of the number, so I used the library function UpnpGetErrorMessage() to do the translation for me:

ErrorMessage = UpnpGetErrorMessage( UpnpGetIfInfo(p, "if0v4") );

and expect the right message string:

//EXPECT_STREQ(str1, str2);
EXPECT_STREQ(ErrorMessage, "UPNP_E_SUCCESS");

or nested all in one:

EXPECT_STREQ(UpnpGetErrorMessage(
              UpnpGetIfInfo(p, "if0v4")),
              "UPNP_E_SUCCESS");

Now it comes to the real work: hunting the side effects that prevent UpnpGetIfInfo() to return "UPNP_E_SUCCESS". It took me some time to examine the code and using the debugger to find that UpnpGetIfInfo() calls system function getifaddrs() one time, freeifaddrs() one time and if_nametoindex one time. I don't count the calls, that tells me googletest. I mocked these functions (and others for UpnpInit2()) so they return given values if called. And giving the right values UpnpGetIfInfo() will return "UPNP_E_SUCCESS" with Unit under Test.

[mrjimenez]

Ok, Ingo, I follow you.

My question is: how does google test initializes the contents of the struct pointed to by p?

In the snip part of the code you posted, one example I can see is that there is no code initializing the log subsystem. And the whole library, UpnpGetIfInfo() included, depends upon this initialization.

From what I can see, what you are doing is allocating memory through UpnpLib_new() and calling UpnpGetIfInfo() without calling UnpnInit2(). Please confirm if that claim is correct. And in that case, do you expect UpnpGetIfInfo() to initialize *p because you mocked some functions? How can this be substitute for a call to UpnpInit2()?

The sequence you show me is this:

1. Mock some functions
2. Call p = UpnpLib_new()
3. Call UpnpGetIfInfo(p, "if0v4")

My claim is that no matter what functions you mock, UpnpGetIfInfo() will receive a fully non-initialized struct through the pointer p, because:

1. No code has any knowledge of *p before UpnpLib_new() has been called;
2. UpnpLib_new() does not initialize *p;
3. The code to initialize *p has not been called before you call UpnpGetIfInfo(p, "if0v4").

The mocking you show would help if UpnpInit2() was being called. My point is that it is not being called and there is no way you can initialize *p without passing *the same `p`** to UpnpInit2().

[ingo-h]

Hi @mrjimenez, to understand things better, first the complete TestSuite for UpnpGetIfInfo():

TEST_F(UpnpApiIPv4TestSuite, UpnpGetIfInfo_called_with_valid_interface)
{
    // provide a network interface
    struct ifaddrs* ifaddr = nullptr;
    CIfaddr4 ifaddr4Obj;
    ifaddr4Obj.set("if0v4", "192.168.99.3/11");
    ifaddr = ifaddr4Obj.get();

    // mock needed system functions
    EXPECT_CALL(mockGetifaddrsObj, getifaddrs(_))
        .WillOnce(DoAll(SetArgPointee<0>(ifaddr), Return(0)));
    EXPECT_CALL(mockFreeifaddrsObj, freeifaddrs(ifaddr))
        .Times(1);
    EXPECT_CALL(mockIf_nametoindexObj, if_nametoindex(_))
        .WillOnce(Return(2));

    UpnpLib* p = UpnpLib_new();

    EXPECT_STREQ(UpnpGetErrorMessage(
                  UpnpGetIfInfo(p, "if0v4")),
                  "UPNP_E_SUCCESS");

    // gIF_NAME mocked with getifaddrs above
    EXPECT_EQ(*UpnpLib_get_gIF_NAME(p), *&"if0v4");
    // gIF_IPV4 mocked with getifaddrs above
    EXPECT_EQ(*UpnpLib_get_gIF_IPV4(p), *&"192.168.99.3");
    EXPECT_EQ(*UpnpLib_get_gIF_IPV4_NETMASK(p), *&"255.224.0.0");
    EXPECT_EQ(*UpnpLib_get_gIF_IPV6(p), *&"");
    EXPECT_EQ(UpnpLib_get_gIF_IPV6_PREFIX_LENGTH(p), (unsigned)0);
    EXPECT_EQ(*UpnpLib_get_gIF_IPV6_ULA_GUA(p), *&"");
    EXPECT_EQ(UpnpLib_get_gIF_IPV6_ULA_GUA_PREFIX_LENGTH(p), (unsigned)0);
    // index mocked with if_nametoindex above
    EXPECT_EQ(UpnpLib_get_gIF_INDEX(p), (unsigned)2);
    EXPECT_EQ(UpnpLib_get_LOCAL_PORT_V4(p), (unsigned short)0);
    EXPECT_EQ(UpnpLib_get_LOCAL_PORT_V6(p), (unsigned short)0);
    EXPECT_EQ(UpnpLib_get_LOCAL_PORT_V6_ULA_GUA(p), (unsigned short)0);

    UpnpLib_delete(p);
}

It is difficult to direct answer your questions because they are asked from a wrong view. You are caught in the traditional view of code programming. Unit test is a tool to develop software. It is not only for testing. From scientific theory a piece of software (UpnpGetIfInfo()) is seen as an isolated unit. It is important to have in mind that it is a Unit under Test. That's not the real world. Now you define conditions that are needed or fail the unit. This way I have verified that UpnpGetIfInfo() will end with "UPNP_E_SUCCESS" with the conditions I have given. If you suspect that this are not the right conditions or not all, then the Unit is not working correct or it is incomplete. Correct the conditions or add the missing and make the Unit to finish again with "UPNP_E_SUCCESS". This will then also correctly work in the real world.

If you look at the TestSuite you will find that UpnpLib* p = UpnpLib_new(); is called and executed by the TestSuite before calling UpnpGetIfInfo(). I had a look at UpnpLib_new() and there is code executed that initialize a structure with defined values (NULL, 0, nullptr, etc. but no dangling pointer that result in Segmentation fault). I cannot say more than that this is enough to finish the function correctly (together with the mocked functions). Sorry, that is fact. If this should not be then it must be corrected.

one example I can see is that there is no code initializing the log subsystem. And the whole library, UpnpGetIfInfo() included, depends upon this initialization.

The whole library consists of different Units.UpnpGetIfInfo() finishes successful without it, nothing more is said. Shouldn't it?

From what I can see, what you are doing is allocating memory through UpnpLib_new() and calling UpnpGetIfInfo() without calling UnpnInit2(). Please confirm if that claim is correct.

Yes, that is correct.

And in that case, do you expect UpnpGetIfInfo() to initialize *p because you mocked some functions?

As verified by the TestSuite UpnpGetIfInfo() is the part that "initialized" *p, not UpnpInit2(). I expect that the function finishes successfully with p = UpnpLib_new() called before. But that was not enough. It also need correct values returned by getifaddrs(), freeifaddrs() and if_nametoindex(). Then googletest confirms my expectation.

How can this be substitute for a call to UpnpInit2()?

Very obviously UpnpInit2() does not need to do anything for UpnpGetIfInfo(). All what it needs is doing by itself, in particular calling getifaddrs(), freeifaddrs() and if_nametoindex(). That is also logical, because UpnpInit2() will get information from UpnpGetIfInfo().

The sequence you show me is this:

1. Mock some functions
2. Call p = UpnpLib_new()
3. Call UpnpGetIfInfo(p, "if0v4")

Yes.

My claim is that no matter what functions you mock, UpnpGetIfInfo() will receive a fully non-initialized struct through the pointer p, because:

1. No code has any knowledge of *p before UpnpLib_new() has been called;

Correct

2. UpnpLib_new() does not initialize *p;

I do not think so. There is code executed that initializes random memory to well defined values so it does not result in segmentation fault.

3. The code to initialize *p has not been called before you call UpnpGetIfInfo(p, "if0v4").

Except UpnpLib_new() there is no more code needed for additional modifying *p.

The mocking you show would help if UpnpInit2() was being called. My point is that it is not being called and there is no way you can initialize *p without passing *the same `p`** to UpnpInit2().

Googletest shows us that it is not needed to call UpnpInit2() to successful finish UpnpGetIfInfo() as Unit under Test.

[mrjimenez]

It is difficult to direct answer your questions because they are asked from a wrong view. You are caught in the traditional view of code programming. Unit test is a tool to develop software. It is not only for testing. From scientific theory a piece of software (UpnpGetIfInfo()) is seen as an isolated unit. It is important to have in mind that it is a Unit under Test. That's not the real world. Now you define conditions that are needed or fail the unit. This way I have verified that UpnpGetIfInfo() will end with "UPNP_E_SUCCESS" with the conditions I have given. If you suspect that this are not the right conditions or not all, then the Unit is not working correct or it is incomplete. Correct the conditions or add the missing and make the Unit to finish again with "UPNP_E_SUCCESS". This will then also correctly work in the real world.

Google tests and mocks run in the real world, and they are implemented through C++. We cannot escape that.

If you look at the TestSuite you will find that UpnpLib* p = UpnpLib_new(); is called and executed by the TestSuite before calling UpnpGetIfInfo(). I had a look at UpnpLib_new() and there is code executed that initialize a structure with defined values (NULL, 0, nullptr, etc. but no dangling pointer that result in Segmentation fault). I cannot say more than that this is enough to finish the function correctly (together with the mocked functions). Sorry, that is fact. If this should not be then it must be corrected.

As I showed you, this is not the case. I have shown you that all the log structures will not be initialized. In particular, I had this problem before, the code worked on linux but did not work on windows, because the pthread mutex implementations are different.

one example I can see is that there is no code initializing the log subsystem. And the whole library, UpnpGetIfInfo() included, depends upon this initialization.

The whole library consists of different Units.UpnpGetIfInfo() finishes successful without it, nothing more is said.

On one system this might be true, but not in all systems. As you seem to like theory, which is a good thing, don't get me wrong, consider this: one example does not confirm a thesis. One counter-example destroys a thesis. The fact that the program works under a certain circumstance does not guarantees its correctness. A million test suits will not do it. A single failing test is enough to show that it does not work.

Just look at the checks. You have failing tests in

1. Build / Build (ubuntu-16.04) - Debug
2. Build / Build (ubuntu-18.04) - Release
3. Build / Build (ubuntu-20.04) - Debug
4. Build / Build (ubuntu-20.04) - Release
5. Build / Build (windows-2016) - Debug
6. Build / Build (windows-2016) - Release
7. Build / Build (windows-2019) - Debug
8. Build / Build (windows-2019) - Release

From what I can see, what you are doing is allocating memory through UpnpLib_new() and calling UpnpGetIfInfo() without calling UnpnInit2(). Please confirm if that claim is correct.

Yes, that is correct.

And in that case, do you expect UpnpGetIfInfo() to initialize *p because you mocked some functions?

As verified by the TestSuite UpnpGetIfInfo() is the part that "initialized" *p, not UpnpInit2().

You just have to look at the code to see that your statement is false.

I expect that the function finishes successfully with p = UpnpLib_new() called before. But that was not enough. It also need correct values returned by getifaddrs(), freeifaddrs() and if_nametoindex(). Then googletest confirms my expectation.

Why are you ignoring UpnpPrintf()? In spite of me telling you that it will use data that is only initialized if UpnpInit2() is called. More precisely, UpnpSetLogFileName() must be called before any log function be used. If you don't do that, you get undefined behavior. And that means that the code may work sometimes and may fail other times.

How can this be substitute for a call to UpnpInit2()?

Very obviously UpnpInit2() does not need to do anything for UpnpGetIfInfo(). All what it needs is doing by itself, in particular calling getifaddrs(), freeifaddrs() and if_nametoindex(). That is also logical, because UpnpInit2() will get information from UpnpGetIfInfo().

Please, follow this: UpnpGetIfInfo() calls UpnpPrintf(). Do you disagree? What part of the code initializes these variables:

struct s_UpnpLib
{
...
        ithread_mutex_t m_gLogMutex;
        Upnp_LogLevel m_gLogLevel;
        FILE *m_gLogFp;
        int m_gLogIsStderr;
        int m_gSetLogWasCalled;
        int m_gLogInitWasCalled;
        char *m_gLogFileName;
};

The sequence you show me is this:

1. Mock some functions
2. Call p = UpnpLib_new()
3. Call UpnpGetIfInfo(p, "if0v4")

Yes.

My claim is that no matter what functions you mock, UpnpGetIfInfo() will receive a fully non-initialized struct through the pointer p, because:

1. No code has any knowledge of *p before UpnpLib_new() has been called;

Correct

1. UpnpLib_new() does not initialize *p;

I do not think so. There is code executed that initializes random memory to well defined values so it does not result in segmentation fault.

It is not a matter o thinking. I wrote that code. It just sets stuff to zero. And this is deliberated so that if you don't call the initializer functions there is a huge chance that the program will segfault soon. But even that is not guaranteed.

1. The code to initialize *p has not been called before you call UpnpGetIfInfo(p, "if0v4").

Except UpnpLib_new() there is no more code needed for additional modifying *p.

Sorry, this is false. I know because I wrote it. Just read the code.

The mocking you show would help if UpnpInit2() was being called. My point is that it is not being called and there is no way you can initialize *p without passing *the same `p`** to UpnpInit2().

Googletest shows us that it is not needed to call UpnpInit2() to successful finish UpnpGetIfInfo() as Unit under Test.

If that is so, then Googletest is wrong.

And as I said before, working once does not prove anything. Failing once does.

I do not wish to proceed in this discussion about variable initialization, since it is leading us nowhere. I just wanted to ask you one more question: from the results of the tests, what is your conclusion?

[ingo-h]

I just wanted to ask you one more question: from the results of the tests, what is your conclusion?

From the results of the tests there is ongoing software development, either by improving the software or to decide that the test meets all known conditions so far. You are requested to improve the test if you meet a condition you have overseen, even after years.

If that is so, then Googletest is wrong.

There is no point in ignoring 20 years of academic work and practical experience with tons of dissertations and teaching at all relevant universities and schools.

[Vollstrecker]

Sweet, he wanted to test the clutch without a motor and because you don't understand that motor is broken if the clutch can't be rotated with a toy-motor that maybe work or not, he is offended like a five year old and dumbs all his other toys, too.

For me it seems like he is an academic straight from the university that now want's to change the world. Strictly like he learned it there, the real world has to be adjusted to the academic one.

Anyways, the basis for using gtest is done, and @AlaricSenat will maybe benefit from that.

[mrjimenez]

Well, I think it is most unfortunate that Ingo closed this issue, I still hoped that the would prove me wrong. Then I would really have learned something new.

I just wanted to ask you one more question: from the results of the tests, what is your conclusion?

From the results of the tests there is ongoing software development, either by improving the software or to decide that the test meets all known conditions so far. You are requested to improve the test if you meet a condition you have overseen, even after years.

I meant objectively, not generically. What did you find out? What has to be changed? Where is the problem? UpnpLib_new() or UpnpGetIfInfo()?

If that is so, then Googletest is wrong.

There is no point in ignoring 20 years of academic work and practical experience with tons of dissertations and teaching at all relevant universities and schools.

I am not ignoring anything. Notice that my sentence begins with an If and concludes with a then. I also took a lot of time to understand your code and what google test is doing, which is the opposite of ignoring. On the other hand, you are ignoring the supreme evidence: code. It is written. It is an indisputable fact. No one can deny what is written. You can only argue about what is really happening and how the compiler interprets it. That is usually the issue with C++.

I hope you can still contribute with the project. I have nothing against good theory and good programming practices.

[ingo-h]

Hi Marcelo,

I am not ignoring anything. [...] I hope you can still contribute with the project. I have nothing against good theory and good programming practices.

I*m glad to read this :-) I have started to contribute with the project to get stable IPv6 support, and yes - maybe a little immodest - to help to boost it up with modern software development methods. In some private projects I have had experience with unit tests with very good results. With your comment:

I do not wish to proceed in this discussion about variable initialization, since it is leading us nowhere.

I haven't seen how to go on with my suggestion. This is one basis of unit tests to get its paradigm of isolation (sorry for theory). It is needed to get always reproducible tests independent of the system it is running on. One of my goals would be to have MS Windows tests running together with Linux tests on Linux just by mocking its system function calls like we do it with the Linux system calls.

Unit Tests are mainly used for two things:

1. Refactoring of old software projects. Here the productive code isn't touched and tests are created that match the given behavior as much as possible. This is where the experience of the original developer is very valuable because he knows the best what his software needs. As you mentioned: "The fact that the program works under a certain circumstance does not guarantees its correctness." That is the reason why unit tests are never finished. If you find a bug or a shortcoming, then you are required to adapt the test. So the tests will become more and more complete over the time. If we have sufficient unit tests, which cover most of the production code, we can optimize the program with the certainty that it will behave unchanged (refactoring).
2. Develop a program. This starts first with a Test and coding is done against this test. There are many reasons given in the literature why this leads to better code. Practical tests have shown that the code is more compact and more stable then (Benefits).

What I'm talking about and have started is Point 1. Here are some more detailed answers to your questions.

I have shown you that all the log structures will not be initialized.

But UpnpGetIfInfo() does not fail without it. The consequence can not be, to say the test is meaningless. Instead the function should be made to fail without initialized log and the test adopted to it.

Just look at the checks. You have failing tests in

Build / Build (ubuntu-16.04) - Debug Build / Build (ubuntu-18.04) - Release --- snip ---

Yes, that is a lack and it shows that the simple tests are not really isolated running. But we have just started with it and it is the wrong focus to insist on the sanity checks at this time. We should make them clean and stable and then decide to add them step by step to the sanity checks.

As verified by the TestSuite UpnpGetIfInfo() is the part that "initialized" *p, not UpnpInit2().

You just have to look at the code to see that your statement is false.

OK, to be more clear: it is enough to call UpnpLib_new() to get a valid pointer and run UpnpGetIfInfo() with it. After it has successfully finished the following is returned:

"if0v4"        by *UpnpLib_get_gIF_NAME(p)
"192.168.99.3" by *UpnpLib_get_gIF_IPV4(p)
"255.224.0.0"  by *UpnpLib_get_gIF_IPV4_NETMASK(p)
""             by *UpnpLib_get_gIF_IPV6(p)
and some more.

If this should not be (but looks good so far) then UpnpGetIfInfo() is working wrong, not the test. B.t.w. it's simple to just try it to look what happens and I myself was amazed that it works.

Why are you ignoring UpnpPrintf()?

Because I didn't know that it is needed. I haven't inspected the code in all details to understand completely how it works. I executed UpnpGetIfInfo() as unit. But thanks for the info. I will have a look at UpnpPrintf() and make a test for it if not static.

It just sets stuff to zero. And this is deliberated so that if you don't call the initializer functions there is a huge chance that the program will segfault soon.

Just to understand the intention of unit tests: with the conditions I have given it is guaranteed not to segfault. Of course the conditions may be false or incomplete and I have to find the correct ones that are more to the reality.

If you like you can reopen this issue. I would also recover the branch. Maybe it is an option just to play a bit with googletest?

[mrjimenez]

Hi Ingo,

As I stated far at the top, if the testing code you add does not make the sanity checks fail, I have nothing against it and in fact I would have already committed your code.

So lets separate the things. We have Github actions and they are used to prevent regressions. We can use google tests for regression prevention, refactoring and program development. Those activities must be separate. That is all I ask.

If things stay separate, I see the refactoring and program testing department as a factory of regression prevention tests. I.e., tests will migrate from refactoring to Github actions as they start to succeed.

Also, if you decide to recover the branch, do notice that you will probably have to rebase it because the code has advanced a bit, in particular the logging subsystem.

[ingo-h]

Hello Marcelo,

I will try to skip the failing tests so we have time to investigate why. I was searching for sanity checks on github help but couldn't find useful information. Do you know where to look how it works on pupnp?

I have already merged the master to the gtests01 branch, not rebased. Does it matter?

[mrjimenez]

I will try to skip the failing tests so we have time to investigate why. I was searching for sanity checks on github help but couldn't find useful information. Do you know where to look how it works on pupnp?

Take a look here: Github Actions.

The file that configures it in pupnp is .github/workflows/ccpp.yml.

I have already merged the master to the gtests01 branch, not rebased. Does it matter?

Hopefully not, but anyway would not be a big problem, maybe Github can't handle the merge automatically, but we can do it by hand.

[mrjimenez]

Hi folks,

Nice work!

...
33/34 Test #33: test-upnp-HttpHeaderList.TwoElements-static .........................................   Passed    0.00 sec
      Start 34: test-upnp-HttpHeaderList.PlentyElements-static
34/34 Test #34: test-upnp-HttpHeaderList.PlentyElements-static ......................................   Passed    0.00 sec

100% tests passed, 0 tests failed out of 28

Total Test time (real) =   0.49 sec

The following tests did not run:
         15 - test-upnp-UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_valid_interface (Disabled)
         16 - test-upnp-UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_unknown_interface (Disabled)
         17 - test-upnp-UpnpApiIPv4TestSuite.initialize_default_UpnpInit2 (Disabled)
         20 - test-upnp-UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_valid_interface-static (Disabled)
         21 - test-upnp-UpnpApiIPv4TestSuite.UpnpGetIfInfo_called_with_unknown_interface-static (Disabled)
         22 - test-upnp-UpnpApiIPv4TestSuite.initialize_default_UpnpInit2-static (Disabled)

Now that we have gtest running and it certainly runs on the sanity tests, what about adding a CMake option to enable the disabled tests to run? That way we could run them on the desktop, but not on Github actions.

Would you have anything against adding such an option? If not, please do.

Regards, Marcelo.

[Vollstrecker]

you can run them from cmd-line. test_upnpapi and test_upnpapi-static are created ans contain all tests. They should have some help param that tells you how you run disabled, or just specific ones.

An explicit option for that would be possible, but then we would either have to modify the source on configure, or splt them out into a separate-file to compile them only with the option. The first option would be ugly, the second one would mean we could easily identify failing ones, in that caser we could just mark them as failing and let them run with the others (I would prefer that one).

[mrjimenez]

I agree that we don't need an option for each test, this we can do by executing them individually as you stated.

The second option you highlighted is more in line with what I had in mind. Running all of the tests together and making it easier to identify the failing ones.

[Vollstrecker]

It's already prepared (you asked somewhere what I mean with WILL_FAIL: exactly this). Someone just has to take the task to split the file and move the stuff that both need into a header.

[ingo-h]

I'm just working on this with the goal that all gtests will also run on all sanity checks so we can enable them step by step. We should not screw at two ends. Please give me some time for a suggestion. Simple gtests only using googletest are running, like the test_template.cpp and test_UpnpHttpHeaderList.cpp. There is only an issue with googlemock.

To run them on the desktop please have a look at gtest/README.