search

ManageIQ / manageiq-cross_repo

ManageIQ cross-repository testing framework
MIT License
3 stars 18 forks source link

Add --script-cmd opt #64

Closed NickLaMuro closed 4 years ago

NickLaMuro commented 4 years ago

Allows configuring a custom script to be executed for running the specs.

In this scenario, if this is specific to a particular TEST_REPO, then this will only work with said repo and the user will have to configure the cross repo run for that test in particular, and leave the others as the default.

This should solve the issues I am having in https://github.com/ManageIQ/manageiq-cross_repo-tests/pull/150 by adding the following to the .travis.yml for that one:

   fast_finish: true
 env:
   global:
-  - REPOS=
+  - REPOS=ManageIQ/manageiq/pull/20350,ManageIQ/manageiq-ui-classic/pull/7201
   matrix:
-  - TEST_REPO=manageiq
+  - TEST_REPO=ManageIQ/manageiq-ui-classic/pull/7201 SCRIPT_CMD="bundle exec rake spec:cypress"
agrare commented 4 years ago

Hey @NickLaMuro, if you're mainly trying to run other rake tasks could you do the same thing with the existing TEST_SUITE env var in the .travis.yml matrix?

Something like

env:
  matrix:
  - TEST_REPO=manageiq-ui-classic#1234 TEST_SUITE=spec
  - TEST_REPO=manageiq-ui-classic#1234 TEST_SUITE=spec:cypress
agrare commented 4 years ago

It failed due to not having nvm installed but it appears to respect the TEST_SUITE https://travis-ci.com/github/ManageIQ/manageiq-cross_repo-tests/builds/176048283

Fryguy commented 4 years ago

Should be easy enough to install nvm. I am +1 for the TEST_SUITE idea for now since it already works and requires no code changes.

I can see the benefit of a SCRIPT_CMD for the future, but seems we don't need it, so probably should wait until we do? @agrare Thoughts?

NickLaMuro commented 4 years ago

@agrare I don't think your test is valid.

The only reason their was a difference in your failing spec is that it failed when doing before_install.sh, which does respect the TEST_SUITE variable in manageiq-ui-classic to determine what is pre-installed as part of that file:

https://github.com/ManageIQ/manageiq-ui-classic/blob/master/tools/ci/before_install.sh#L2

Which I change in my PR to include spec:cypress.

The command that would still be run is still going to be bundle exec rake since that is hard coded in bundle exec manageiq-cross_repo:

Failed

Passing

Edit: In addition, for your other manageiq-ui-classic matric usesspec` as the example, which is just the default in the rake file:

https://github.com/ManageIQ/manageiq-ui-classic/blob/4a48770a103e1c6533fd6ce73e13759fdb38a1ae/Rakefile#L83

That said, what your script did show is that I might need some other changes as well to make this work... 😩

cc @Fryguy

NickLaMuro commented 4 years ago

I am confused why nvm wasn't found in the first place:

https://travis-ci.com/github/ManageIQ/manageiq-cross_repo-tests/jobs/362121956#L100-L101

Actually... it probably is because we are invoking the before_install.rb with just a vanilla bash instead of what we do in travis normally, which is source it into the existing shell.

Probably can be fixed by changing it to bash -l... will try that out on my cross-repo PR.

Edit: Testing some fixes over in my original manageiq-cross_repo-tests PR.

agrare commented 4 years ago

Yeah we would have to change what we run to bundle exec rake $TEST_SUITE but this matches what we do in a lot of repos already so that seems useful. I do agree that something like a TEST_CMD which defaults to the normal bundle exec rake could be useful for runing things other than take tasks

NickLaMuro commented 4 years ago

other than take tasks

/me giggles to himself

*ahem* excuse me...

So as my "Edit:" in my previous comment shows, I updated my existing manageiq-cross_repo-tests PR to test the changes I am considering:

https://github.com/ManageIQ/manageiq-cross_repo-tests/pull/150

A few things of note:

But in addition to just changing the command to be bundle exec rake spec:cypress, I also have to preface it to include bash -l tools/ci/before_script.sh so the assets are compiled and yarn is run.

If I didn't include the SCRIPT_CMD (which I forgot the first time around), the run doesn't get stuck on trying to install nvm, but still runs bundle exec rake spec instead of what I wanted:

https://travis-ci.com/github/ManageIQ/manageiq-cross_repo-tests/builds/176085544

This has been fixed (hopefully) in a build that is currently running:

https://travis-ci.com/github/ManageIQ/manageiq-cross_repo-tests/builds/176087199

🤞

NickLaMuro commented 4 years ago

That moment when you realize you were using the wrong "repo name syntax" the whole time...

https://github.com/ManageIQ/manageiq-cross_repo-tests/pull/150/commits/592eb5748159c096597b3fe06797c2151f4cffca

😩

Fryguy commented 4 years ago

@NickLaMuro Just realized this didn't update the README docs...can you do that when you have a chance?

NickLaMuro commented 4 years ago

@Fryguy https://github.com/ManageIQ/manageiq-cross_repo/pull/65