openSUSE / gitarro

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

Fix: Check repository dir apart from the base git dir #60

Closed meaksh closed 7 years ago

meaksh commented 7 years ago

This PR should fix an issue happening when certain repository has failed to be cloned.

On these cases, the following runs of gitarro would produce the next traceback during execution:

[manager-prs-susemanager-unittests] $ /bin/sh -xe /tmp/jenkins4518559196771098257.sh
+ /home/jenkins/bin/gitbot/gitarro.rb -r SUSE/spacewalk -c susemanager_unittests -d 'python backend pgsql unit test' -t /home/jenkins/jenkins-build/workspace/manager-prs-susemanager-unittests/gitbot_susemanager_unittests/spacewalk/susemanager-utils/testing/automation/susemanager-unittest.sh -u https://ci.suse.de/job/manager-prs-susemanager-unittests/4876/ -f susemanager/ -g /home/jenkins/jenkins-build/workspace/manager-prs-susemanager-unittests/gitbot_susemanager_unittests
/home/jenkins/bin/gitbot/lib/gitarro/git_op.rb:68:in `chdir': No such file or directory @ dir_chdir - spacewalk (Errno::ENOENT)
    from /home/jenkins/bin/gitbot/lib/gitarro/git_op.rb:68:in `rescue in goto_prj_dir'
    from /home/jenkins/bin/gitbot/lib/gitarro/git_op.rb:63:in `goto_prj_dir'
    from /home/jenkins/bin/gitbot/lib/gitarro/git_op.rb:28:in `merge_pr_totarget'
    from /home/jenkins/bin/gitbot/lib/gitarro/backend.rb:24:in `pr_test'
    from /home/jenkins/bin/gitbot/lib/gitarro/backend.rb:137:in `launch_test_and_setup_status'
    from /home/jenkins/bin/gitbot/lib/gitarro/backend.rb:121:in `reviewed_pr_test'
    from /home/jenkins/bin/gitbot/gitarro.rb:28:in `block in <main>'
    from /home/jenkins/bin/gitbot/gitarro.rb:12:in `each'
    from /home/jenkins/bin/gitbot/gitarro.rb:12:in `<main>'
...

The PR changes the behavior to actually check if the repository path exists. (not only the base git path).

/cc @MalloZup

meaksh commented 7 years ago

@MalloZup - I don't know why rubocop is not happy here while he's having a good time locally..

juliogonzalez commented 7 years ago

@meaksh, we can ignore rubocop this time. It's a problem from master. @MalloZup is working on it.

MalloZup commented 7 years ago

@meaksh thx LGTM for me. @juliogonzalez maybe we want to rebase the 2 commits? do we had a policiy for that ? :wolf:

juliogonzalez commented 7 years ago

Despite the command is rebase, in this context is best to say "squash".

And if my opinion counts... a PR should only have one commit so we can have the git history clean and easy to read :-)

juliogonzalez commented 7 years ago

BTW @meaksh, master is fixed now, so if you rebase the commits from master you will see travis running all the tests without any problem.

meaksh commented 7 years ago

@juliogonzalez - Rebased. Let's wait for rubocop 😄

MalloZup commented 7 years ago

@meaksh thx if rubocop is happy we can go with ti, ! thx for the handy fix!

juliogonzalez commented 7 years ago

@meaksh Could you squash the commits into a single one? :-)

meaksh commented 7 years ago

Ha! @juliogonzalez - Squashing ;-)

juliogonzalez commented 7 years ago

Thanks @meaksh! :-)

@MalloZup ready to merge as soon as the bot is happy about it.