openSUSE / gitarro

run all your test against a GitHub Pull request
https://opensuse.github.io/gitarro
MIT License
15 stars 20 forks source link

Do not exit 1 when --check is enabled, but show a message #78

Closed juliogonzalez closed 7 years ago

juliogonzalez commented 7 years ago

What does this PR do?

Do not exit 1 when --check is enabled, but show a message.

What issues does this PR fix or reference?

Tests written?

Yes, two now rspec.

Workflow:

Followed

juliogonzalez commented 7 years ago

Clarification: I know that the patch only writes something when the PR requires tests, and it should print TESTRQ=false when it does not. But with the current workflow it introduces too much complexity and I am afraid it will required a refactor.

Let's refactor backend.rb and gitarro.rb on a separate PR so we have a single call to launch_test_and_setup_status() and a single exit() using its outcome.

MalloZup commented 7 years ago

before refactoring , we need 2 things.

  1. remove CHECK option 2 remove Changelog option

you are modifying the CHECK option, before refactoring i would just remove this check option and make 2;

  1. (official gem) gitarro ( test_executor)

  2. (tools) (gitarro_check) we need an open isssue gitarro_check would be only a jenkins utilty/tool, that trigger other jobs. `gitarro_check ( tests if there is CONTEXT already present)

  3. (tools) (gitarro_changelog)

juliogonzalez commented 7 years ago

Let's do that change at another PR.

Right now I am only changed the exit value to 0 and printing a message we have a coherent return value (0 unless we have errors) and Jenkins can just grep the output and see if any PR requires tests (so the trigger can call the actual test job).

juliogonzalez commented 7 years ago

Right now we need to fix the problem with the limits, since most tests are just not working because of that.

Then I suggest we create a tag, so gitarro is cloned (--depth 1 --branch ) each time a Jenkins job needs it (and it's not affected by salt as now) and we can keep developing without so much fear.

And then we can start refactoring.

MalloZup commented 7 years ago

sure , we need only the issue for the check

MalloZup commented 7 years ago

https://github.com/openSUSE/gitarro/issues/79 ( this will be better adressed on this issue)

juliogonzalez commented 7 years ago

Ready:

Finished in 2 minutes 58.2 seconds (files took 0.17468 seconds to load)
11 examples, 0 failures
MalloZup commented 7 years ago

after travis merging