git-review should be tested

Bug #1048724 reported by Andrew Hutchings
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
git-review
Fix Released
Medium
Dina Belova

Bug Description

git-review currently does direct SSH access. We should instead use gerritlib. One main advantage of this is it makes mock based testing a lot easier.

Changed in git-review:
assignee: nobody → matthew wagoner (matthew-wagoner)
importance: Undecided → High
status: New → Triaged
Revision history for this message
Marcin Cieślak (saperski) wrote :

I'd say it's a long way to and not a very high priority:

1. gerritlib uses paramiko, which does not support SCP (yet) and we need this to install hook (but please see mine https://github.com/saper/simple-scp-client)

2. no value added as there is no support to HTTPS instead of SSH (as wished by some users here, see bug 1090118 or bug 1021073)

Revision history for this message
Chmouel Boudjnah (chmouel) wrote :

I think the value add is to be able to unit tests git-review which is pretty much inexistent atm. I like your imp of a simple scp client tho, nice work.

Revision history for this message
Jeremy Stanley (fungi) wrote :

Yes, effectively we want something in git-review we can cleanly mock for unit testing. As discussed in https://review.openstack.org/17552 mocking Gerrit itself is mostly impossible (leading to hackish scripts like https://github.com/fungi/grtest/blob/master/grtest for regression testing).

Revision history for this message
Jeremy Stanley (fungi) wrote :

From a unit testing perspective, I've decided to mock the shell interactions instead (git-review's I/O is really all done via shell invocation of ssh, git, et cetera). Thus the testing improvements are no longer driving this bug's priority.

Changed in git-review:
importance: High → Low
assignee: =w= (zxkuqyb) → nobody
Jeremy Stanley (fungi)
summary: - git-review should use gerritlib
+ git-review should be tested
Changed in git-review:
importance: Low → Medium
Changed in git-review:
assignee: nobody → Jeremy Stanley (fungi)
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to git-review (master)

Reviewed: https://review.openstack.org/35104
Committed: http://github.com/openstack-infra/git-review/commit/e99aa8b16db91c9829848c5b21f471d646c0b93f
Submitter: Jenkins
Branch: master

commit e99aa8b16db91c9829848c5b21f471d646c0b93f
Author: Dina Belova <email address hidden>
Date: Mon Jul 1 12:49:24 2013 +0400

    Implement integration tests.

    Provide intergation testing for the git-review utility. Requires
    Gerrit war file (like 2.6.1-gerrit.war) in the .gerrit directory in
    the git-review project (like git-review/.gerrit/2.6.1-gerrit.war).

    Tests start local Gerrit and create test user and project there to
    be used.

    Closes-Bug: 1048724
    Change-Id: I3242479dcbcf230085178004540992680f3f8e30

Changed in git-review:
status: In Progress → Fix Committed
Jeremy Stanley (fungi)
Changed in git-review:
assignee: Jeremy Stanley (fungi) → Dina Belova (dbelova)
Revision history for this message
Jeremy Stanley (fungi) wrote :

It looks like we're still hitting false negative results with this new test suite, partial attempts to work around it are at https://review.openstack.org/#/q/project:openstack-infra/git-review+branch:master+topic:use-bc,n,z

Revision history for this message
Dina Belova (dbelova) wrote :

Ok, guys, I'll take a look on what's going on with all this staff.

Revision history for this message
Tim Landscheidt (scfc) wrote :

I think I'm encountering false negatives: https://review.openstack.org/#/c/50401/ (current master) has Jenkins pass (http://logs.openstack.org/01/50401/4/gate/gate-git-review-python26/ccad41e) and fail (http://logs.openstack.org/01/50401/4/check/gate-git-review-python26/1fb3235) for patch set #4. So I'm not sure if Jenkins's failures on my own small changeset (https://review.openstack.org/#/c/64307/) are true regressions or just flukes.

I've looked at https://wiki.openstack.org/wiki/GerritJenkinsGit#Test_Failures that suggests errors in test suites are noted at http://status.openstack.org/rechecks/, but no bug is listed there affecting git-review. What's the current status of the test suite?

Revision history for this message
Jeremy Stanley (fungi) wrote :

Unchanged. Just recheck against this bug for now if you hit SSH errors or test timeouts on git-review integration tests.

Jeremy Stanley (fungi)
Changed in git-review:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.