Note that I decided I won't migrate any test using this, until this feature (in this or subsequent form) is accepted upstream. Makes the most sense to me :) This feature can be tested with downstream git, so not problem on that side. Might be there are some valid objections I do not see now ....

I attempted to use the modified role by running the playbook against a target docker container and vagrant host. In both cases, the role failed when attempting to execute the resolve-test-dependencies script because it didn't exist. This makes sense, because the playbook itself is actually running on the control host, but it runs the shell command on the remote system under test--which doesn't have the role files present. I think that maybe the ansible script module is what needs to be used so the script gets transferred to the remote system. Care needs to be taken to get any associated environment variables passed along, too.

TASK [standard-test-rhts : Resolve test dependencies] *****************************************************************************************************************
failed: [14d73588b3ff010799b70ef2ac78b716a1bd7b8141bc526547184000e459ad73] (item=httpd-php-mysql-sanity-test) => {"changed": true, "cmd": "/home/merlinm/pagure/_fork/mvadkert/standard-test-roles/roles/standard-test-rhts/files/resolve-test-dependencies httpd-php-mysql-sanity-test", "delta": "0:00:00.110196", "end": "2017-09-15 20:48:02.319064", "failed": true, "item": "httpd-php-mysql-sanity-test", "rc": 127, "start": "2017-09-15 20:48:02.208868", "stderr": "/bin/sh: /home/merlinm/pagure/_fork/mvadkert/standard-test-roles/roles/standard-test-rhts/files/resolve-test-dependencies: No such file or directory", "stderr_lines": ["/bin/sh: /home/merlinm/pagure/_fork/mvadkert/standard-test-roles/roles/standard-test-rhts/files/resolve-test-dependencies: No such file or directory"], "stdout": "", "stdout_lines": []}
        to retry, use: --limit @/home/merlinm/upstreamtest/httpd/tests.retry

Perhaps the -w option should also be passed to grep to prevent partial matches in the processed libs.

I would suggest using a slightly better delimited patter to replacement than simply COMPONENT. Perhaps _COMPONENT_ or similar? Adjust GIT_BASE_URL above and in the environment as well.

I don't know how long these lists might get, but xargs limits the line length--which would mess things up. It might be safer to replace that final xargs in the above pipelines with something like tr '\n' ' ' instead.

This probably needs to use the ansible script module instead, so the resolve-test-dependencies is transferred to the remote machine before it gets run. Be sure any required environment variables are handled as well.

I attempted to use the modified role by running the playbook against a target docker container and vagrant host. In both cases, the role failed when attempting to execute the resolve-test-dependencies script because it didn't exist. This makes sense, because the playbook itself is actually running on the control host, but it runs the shell command on the remote system under test--which doesn't have the role files present. I think that maybe the ansible script module is what needs to be used so the script gets transferred to the remote system. Care needs to be taken to get any associated environment variables passed along, too.
TASK [standard-test-rhts : Resolve test dependencies] *****************
failed: [14d73588b3ff010799b70ef2ac78b716a1bd7b8141bc526547184000e459ad73] (item=httpd-php-mysql-sanity-test) => {"changed": true, "cmd": "/home/merlinm/pagure/_fork/mvadkert/standard-test-roles/roles/standard-test-rhts/files/resolve-test-dependencies httpd-php-mysql-sanity-test", "delta": "0:00:00.110196", "end": "2017-09-15 20:48:02.319064", "failed": true, "item": "httpd-php-mysql-sanity-test", "rc": 127, "start": "2017-09-15 20:48:02.208868", "stderr": "/bin/sh: /home/merlinm/pagure/_fork/mvadkert/standard-test-roles/roles/standard-test-rhts/files/resolve-test-dependencies: No such file or directory", "stderr_lines": ["/bin/sh: /home/merlinm/pagure/_fork/mvadkert/standard-test-roles/roles/standard-test-rhts/files/resolve-test-dependencies: No such file or directory"], "stdout": "", "stdout_lines": []}
to retry, use: --limit @/home/merlinm/upstreamtest/httpd/tests.retry

good point, we will look @ this, the new patch is almost ready

Other issues were resolved in the rebased patch, namely

1. we added the -w option for the grep (thanks!)
2. the download of roles from git location was removed and the role will use the packaged beakerlib-libraries only
3. xargs has been replaced as advised

Thanks very much for the comments and sorry it took so long to rework :(

Will update once the final version is ready

@merlinm @bgoncalv this PR is finally ready for review. Please see the updated Description for up-to-date information about how to run it.

FTR, all your comments should be addressed now! I tested the role also from outside and should work. Note that the standard-test-rhts role currently supports only classic environment.

@mvadkert is it planned to have beakerlib-libraries shipped on Fedora official repo? In that case is this patch just a temporary fix?

@mvadkert is it planned to have beakerlib-libraries shipped on Fedora official repo? In that case is this patch just a temporary fix?

@bgoncalv Yes, that is correct. I will add another PR once the upstream fix gets into the repositories.

@merlinm so waiting for your review, no additional patches needed.

This should have when: use_beakerlib_libraries

Since you changed this to use synchronize, please add the ssh_args: "-o UserKnownHostsFile=/dev/null" parameter just like the other synchronize task in this file. Otherwise, the public key of the system under test will be recorded in ssh's known_hosts file--which can cause a conflict if another disposable test system appears with the same IP address but different public key.

@merlinm thanks Merlin! Addressed both things and rebased on current master!

I might be missing something, from what I understood the only thing that we need is to be able to install beakerlib-libraries and therefore we just need to enable the copr repo.

I've imagined that just adding the code below would be enough (similar change for the rhts role...), and the project that needs beakerlib-libraries would just include it as require package on its tests.yml.

--- a/roles/standard-test-beakerlib/tasks/main.yml
+++ b/roles/standard-test-beakerlib/tasks/main.yml
@@ -11,10 +11,21 @@
    setup:
    delegate_facts: True
+  - name: Enable COPR repo for beakerlib-libraries
+    shell: dnf copr -y enable mvadkert/beakerlib-libraries

@bgoncalv note that what my patch does it also "dynamically" resolves dependencies of the beakerlib-libraries. So your approach would require to done this in the tests itself.

@merlin ping? :) I explained everything to Bruno btw, he should have no more concerns I thing. @bgoncalv right?

Right, it is all good to me :-)

Code looks good.

I checked to make sure this PR didn't break existing tests using standard-test-rhts without enabling use_beakerlib_libraries. Good there.

I tried running the nfs-utils tests mentioned in the original PR description that enable use_beakerlib_libraries. They ran, but appeared to hang. I was advised there's still some bugs in those tests, so I limited to running just the 'mount' tests and that worked as expected.

LGTM.