QA » openQA Project

mkittler wrote:

Still missing:

• Targets for installing openvswitch
  • This should be directly supported by the CMake build script because it should make use of variables such as CMAKE_INSTALL_PREFIX to achieve a consistent installation.
• Targets for tidying C++ and Perl files
  • It is not hard to invoke the scripts by hand but targets would be nice to have.
• Targets for invoking the Perl testsuite and computing coverage
  • I'm not sure whether it makes sense to have these within CMake.
    • pros: These targets rely on a successful built so it would of course be nice if that dependency would be tracked within CMake. It is also a bit weird to use a different Makefile for testing. CTest also has some nice features, e.g. it allows to include/exclude tests conveniently using a regex.
    • cons: There are a lot of targets to consider and it might take a while until everything is in place.

Having consistent dependency handling would definitiely be nice to have.

Adding these gradually seems fine to me anyway. We're also leaving the "old" ones in place for now, right? My concern if any would be that we're once again diverging from the commands used in openQA and this causes needless, sublte confusion all the time (like TESTS= assuming a different folder, different verbosity defaults etc.)

We're also leaving the "old" ones in place for now, right?

Right.

My concern if any would be that we're once again diverging from the commands used in openQA

CMake has of course a different usage than automake so breaking everyones build script is unavoidable when we finally remove the automake build script at some point. But we're switching from one standard build system to another and removing a lot of rough edges by the way so I guess that's ok.

target_link_options must not be used yet, see PR description https://github.com/os-autoinst/os-autoinst/pull/1473

Install target for openvswitch files and fix for target_link_options: https://github.com/os-autoinst/os-autoinst/pull/1475

PR for running tests and documentation has been merged: https://github.com/os-autoinst/os-autoinst/pull/1479

Only details like targets for tidying C++ and Perl files are missing. But before tweaking such details it would likely be more interesting to test the CMake build system within Travis and OBS.

PR for using CMake within the CI and OBS: https://github.com/os-autoinst/os-autoinst/pull/1482

I saw that you added build instructions to README but the README also references INSTALL.asciidoc which explains the autotools way. Also you have removed the call of autotools based build so it is not tested anymore. As I would prefer to not remove autotools support right away but give it a bit of time I suggest you add back a (very simple) build-check of the autotools way and merge the build instructions from README with INSTALL.asciidoc

I've missed INSTALL.asciidoc indeed. Seems like this file hasn't been updated for a while, e.g. it suggests to install the openQA package for installing dependencies (although os-autoinst-devel would be the right package now). So likely the file deserves a general update and parts with are redundant to the main README.md or the openQA documentation should be removed.

As I would prefer to not remove autotools support right away but give it a bit of time I suggest you add back a (very simple) build-check of the autotools way

Right, a simple build check within our CI would still be nice.

I've also created a PR to move openQA to CMake: https://github.com/os-autoinst/openQA/pull/3283

However, we could actually not merge it and use is as an occasional check for the autotools build system.

I've tested the CMake-based RPM on openqaworker11 now (with https://github.com/os-autoinst/os-autoinst/pull/1484) and it basically works. I couldn't execute a full test run because something seems wrong with worker setup.

mkittler wrote:

I've missed INSTALL.asciidoc indeed. Seems like this file hasn't been updated for a while, e.g. it suggests to install the openQA package for installing dependencies (although os-autoinst-devel would be the right package now). So likely the file deserves a general update and parts with are redundant to the main README.md or the openQA documentation should be removed.

As I would prefer to not remove autotools support right away but give it a bit of time I suggest you add back a (very simple) build-check of the autotools way

Right, a simple build check within our CI would still be nice.

I've also created a PR to move openQA to CMake: https://github.com/os-autoinst/openQA/pull/3283

However, we could actually not merge it and use is as an occasional check for the autotools build system.

I think we should use cmake build based environments in openQA as well to benefit from that. However as long as we keep automake we should have a test for that. As suggested in the meeting 2020-07-28 I suggest as the first step take a look into the packages in https://build.opensuse.org/package/show/devel:openQA/os-autoinst that currently do not build and try to fix them, maybe even with using automake in these cases which gives us automatic test coverage, e.g. SLE_12_SP5 finds no "ninja". As an alternative add an automake based in travis CI as parallel job

PR for using/testing autotools within SLE12 builds (merged, x86_64 build succeeds on devel:openQA): https://github.com/os-autoinst/os-autoinst/pull/1488
PR for documentation: https://github.com/os-autoinst/os-autoinst/pull/1489

What's remaining is using CMake within openQA's CI. That will be a little bit tricky: https://github.com/os-autoinst/openQA/pull/3283#issuecomment-666318629

Is it intentional that os-autoinst travis-ci is now running tests twice? Once without coverage and then with?

@tinita That's a bug. This PR should fix it as well as some of your other comments from the chat: https://github.com/os-autoinst/os-autoinst/pull/1498