QA » openQA Project

I had started already here but as I recurringly fail to finish I am splitting this up in multiple PRs for easier testing and planned merges one at a time:

• DONE https://github.com/os-autoinst/os-autoinst/pull/1793 - t/data, merged
• DONE https://github.com/os-autoinst/os-autoinst/pull/1792 - t, merged
• DONE https://github.com/os-autoinst/os-autoinst/pull/1794 - tools, merged
• DONE https://github.com/os-autoinst/os-autoinst/pull/1791 - OpenQA/, merged
• https://github.com/os-autoinst/os-autoinst/pull/1789 - consoles, multiple errors, needs work but should be merged later anyway
• https://github.com/os-autoinst/os-autoinst/pull/1790 - backend, same as above

let's see which ones succeed. Based on criticality we can decide which ones to merge first. I suggest to first do low-risk ones like tools and t/, later backends and consoles

EDIT: Updated current status of PRs

3 done, 3 to go

additional PR for myjsonrpc only: https://github.com/os-autoinst/os-autoinst/pull/1827 - merged

I now merged https://github.com/os-autoinst/os-autoinst/pull/1790 "use signatures in os-autoinst backend" as it was ready and has approval. I will have alert duty this week so hopefully I would see any fallout :)

There was no feedback on the last merged pull request so I assume we are good and can continue with the next, i.e. "consoles"

okurz wrote:

There was no feedback on the last merged pull request so I assume we are good and can continue with the next, i.e. "consoles"

https://github.com/os-autoinst/os-autoinst/pull/1868

I am playing it safe and doing the first part of consoles with https://github.com/os-autoinst/os-autoinst/pull/1868 (merged). https://github.com/os-autoinst/os-autoinst/pull/1789 includes the second part. I realized that a good additional test is to execute the openQA full stack test while I have my signatures changes in os-autoinst checked out locally. Seems like maybe the os-autoinst tests do not cover the complete interface for the interactive mode and such so I will test my changes with openQA first.

surprisngly cdywan merged https://github.com/os-autoinst/os-autoinst/pull/1789 . I assume they missed our discussions and plans. Regardless, I checked jobs on https://openqa.opensuse.org/tests and found no related failures so I guess we are good with that change. I can now continue with https://github.com/os-autoinst/os-autoinst/pull/1773 but that's still a rather big PR so maybe split out more files.

okurz wrote:

surprisngly cdywan merged https://github.com/os-autoinst/os-autoinst/pull/1789 . I assume they missed our discussions and plans. Regardless, I checked jobs on https://openqa.opensuse.org/tests and found no related failures so I guess we are good with that change.

2 approvals, 2 other reviewers, no objections after having brought up concerns with coverage. I sanity-checked also. Typically we merge in such cases. If you clearly didn't want it merged I suggest a blocking review, draft or not-ready label for the future.

I can now continue with https://github.com/os-autoinst/os-autoinst/pull/1773 but that's still a rather big PR so maybe split out more files.

How about consoles+backends and api(mmapi/bmwqemu/distribution) or something along those lines?

cdywan wrote:

okurz wrote:

surprisngly cdywan merged https://github.com/os-autoinst/os-autoinst/pull/1789 . I assume they missed our discussions and plans. Regardless, I checked jobs on https://openqa.opensuse.org/tests and found no related failures so I guess we are good with that change.

2 approvals, 2 other reviewers, no objections after having brought up concerns with coverage. I sanity-checked also. Typically we merge in such cases. If you clearly didn't want it merged I suggest a blocking review, draft or not-ready label for the future.

sure, not your fault at all but mine and it seems all turned out well.

I can now continue with https://github.com/os-autoinst/os-autoinst/pull/1773 but that's still a rather big PR so maybe split out more files.

How about consoles+backends and api(mmapi/bmwqemu/distribution) or something along those lines?

yes, that's what I am thinking.

one more step: https://github.com/os-autoinst/os-autoinst/pull/2006 merged

https://github.com/os-autoinst/os-autoinst/pull/2046 for commands.pm . Should wait a day or two for feedback before continuing.

Brought up in weekly 2022-05-13. I was making a stupid mistake on trying to just add -signatures in one commit and see how that goes but of course that stumbles already over the prototypes used. So I should definitely use both together.

I am welcoming everybody to review https://github.com/os-autoinst/os-autoinst/pull/1696 and help me with some suggestions.

https://github.com/os-autoinst/os-autoinst/pull/2060 merged including parts of the above including a syntax check to ensure we have signatures in all perl files, except testapi.pm so far.

As this PR still turns out to be too big to be easily review I am splitting out yet another PR:

• https://github.com/os-autoinst/os-autoinst/pull/2069

To prevent breakage in os-autoinst-distri-opensuse I prepared three PRs:

• https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/14996
• https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/14997
• https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/14998

All three merged. Now I created https://github.com/os-autoinst/os-autoinst/pull/2078 which should include only the testapi.pm changes that are covered by automatic tests.

okurz wrote:

https://github.com/os-autoinst/os-autoinst/pull/1696 . Haven't found anymore problems on openqaworker7. I suggest we continue with some unit tests in a mob programming session.

testapi unit tests, as opposed to #94952 which covers basetest

https://github.com/os-autoinst/os-autoinst/pull/2252

https://github.com/os-autoinst/os-autoinst/pull/2261