Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#4454 enhancement closed fixed (fixed)

Lucid Lynx (Ubuntu 10.04) buildslaves are failing

Reported by: Glyph Owned by:
Priority: highest Milestone:
Component: conch Keywords:
Cc: Branch: branches/openssh-disconnect-compat-4454
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

As shown here, we have a few tests which are failing on the Lucid buildslaves.

Given that this is a pretty important supported platform for Twisted, we should investigate.

This looks like an incompatibility with a new version of openssh, however, and while this is a potentially serious bug, it's not a regression, because nothing changed on our side of things.

I would like to suggest that we mark the failing tests as .todo or .skip based on a version check of openssh (pointing at a different ticket, of course) and then close this ticket. We should skip only against the precise version found in lucid, until the problem can be properly diagnosed, so we don't have commits to unrelated code generating spurious failing builds.

Change History (11)

comment:1 Changed 12 years ago by Glyph

Priority: normalhighest

comment:2 Changed 12 years ago by Jean-Paul Calderone

Since todos never get fixed, I'm not really in favor of that suggestion.

comment:3 in reply to:  2 Changed 12 years ago by Glyph

Replying to exarkun:

Since todos never get fixed, I'm not really in favor of that suggestion.

99% of our todos (which never get fixed) are in limbo, with no ticket pointing at them.

I'm suggesting filing a ticket to fix the tests, and only skipping the tests on this one particular version of openssh until we know what's going on.

comment:4 Changed 12 years ago by z3p

Judging from the logs, it appears that the new version of OpenSSH is sending a DISCONNECT message before it drops the connection, which is new behavior. Since the tests aren't expecting that message, it's treated as an error.

Unfortunately I'm going on vacation starting Wednesday, so I'm not going to have time in the near future to address this.

comment:5 Changed 12 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/openssh-disconnect-compat-4454

(In [29031]) Branching to 'openssh-disconnect-compat-4454'

comment:6 Changed 12 years ago by Jean-Paul Calderone

(In [29032]) Weaken the assertion in receiveError so that a disconnect error is allowed

refs #4454

comment:7 Changed 12 years ago by Jean-Paul Calderone

Keywords: review added
Owner: z3p deleted

The tests that are failing on the Lucid slaves are:

  • twisted.conch.test.test_cftp.TestOurServerSftpClient.test_extendedAttributes
  • twisted.conch.test.test_conch.OpenSSHClientTestCase.test_exec

They both fail the same way, and though they usually fail, they don't always. Based on Paul's comment above, I've changed the tests so they they allow (but do not require) a DISCONNECT_BY_APPLICATION message before the connection is closed.

Build results

comment:8 Changed 12 years ago by Glyph

Keywords: review removed
Owner: set to Jean-Paul Calderone

A comment in receiveError explaining the intended effect of this assertion would be helpful, since as you say it is allowed but not required. ("If there is an error, it must be this kind, but there isn't always an error, due to a change in behavior in version X.X.X of OpenSSH") Please add this and merge.

I really wish I could give you a hard time about using unittest.assertEquals here, but after a few minutes investigating what it would take to use something non-deprecated I gave up. We will just have to trick z3p into attending another sprint soon.

comment:9 Changed 12 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [29037]) Merge openssh-disconnect-compat-4454

Author: exarkun Reviewer: glyph Fixes: #4454

Change several Conch tests which communicate with the system install of OpenSSH to accept a possible DISCONNECT_BY_APPLICATION message rather than failing when this is received. Newer versions of OpenSSH send this before they close the connection.

comment:10 Changed 12 years ago by Glyph

This was a duplicate of #4413 and #4425, which are both now closed.

comment:11 Changed 11 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.