#1342 assigned enhancement

rename tests of packaging and improve them to avoid spurious system-dependent test failures

Reported by: zooko Owned by: davidsarah
Priority: minor Milestone: soon
Component: packaging Version: 1.8.1
Keywords: cleanup Cc: zooko
Launchpad Bug:

Description

This is a ticket for me to manage small clean-up patches which I would like to see applied but not before 1.8.2.

Attachments (6)

tighter-catch.darcs.patch (1.7 KB) - added by zooko at 2011-01-28T15:03:39Z.
improved-package-tests.darcs.patch (7.9 KB) - added by zooko at 2011-01-30T15:24:33Z.
default-alias-cleanup.darcs.patch (29.3 KB) - added by davidsarah at 2011-01-30T17:28:02Z.
scripts/common.py: don't assume that the default alias is always 'tahoe' (it is, but the API of get_alias doesn't say so). refs #1342
docs-running-no-install.darcs.patch (30.0 KB) - added by davidsarah at 2011-01-31T01:50:21Z.
docs/running.html: reference quickstart.html instead of install.html, and don't refer to 'installing' Tahoe-LAFS (which is not what the quickstart instructions say to do). refs #1342
relnotes.darcs.patch (29.9 KB) - added by davidsarah at 2011-02-04T20:53:22Z.
relnotes.txt: forseeable -> foreseeable. Don't claim to work on Cygwin (which has been untested for some time).
docfix.darcs.patch (3.8 KB) - added by arch_o_median at 2011-03-22T05:26:43Z.

Download all attachments as: .zip

Change History (34)

Changed at 2011-01-28T15:03:39Z by zooko

comment:1 Changed at 2011-01-28T15:05:08Z by zooko

  • Keywords cleanup review-needed added

Changed at 2011-01-30T17:28:02Z by davidsarah

scripts/common.py: don't assume that the default alias is always 'tahoe' (it is, but the API of get_alias doesn't say so). refs #1342

comment:4 Changed at 2011-01-30T17:49:06Z by davidsarah

Reviewing attachment:improved-package-tests.darcs.patch :

test-dont-install-newer-dep-when-you-already-have-sufficiently-new-one.py is way too long a filename. Also the test is about which dist is built and tested (by setup.py test) when an existing one is installed, not about whether the new dist is installed. (It should be "dist", not "dep", since we're talking about a particular version/build.)

How about test-be-satisfied-with-new-enough-dist.py?

test-dont-use-too-old-dep.py (which should be test-dont-use-too-old-dist.py) has this:

# The goal is to turn red if the build system tries to use the
# source dist when it could have used the binary dist.

which I think is a stale comment from a different test.

Changed at 2011-01-31T01:50:21Z by davidsarah

docs/running.html: reference quickstart.html instead of install.html, and don't refer to 'installing' Tahoe-LAFS (which is not what the quickstart instructions say to do). refs #1342

comment:5 Changed at 2011-02-02T22:05:16Z by zooko

  • Cc zooko added

comment:6 Changed at 2011-02-04T06:31:25Z by davidsarah

Given that #1355 is a potential regression, we probably shouldn't apply these quite yet.

Changed at 2011-02-04T20:53:22Z by davidsarah

relnotes.txt: forseeable -> foreseeable. Don't claim to work on Cygwin (which has been untested for some time).

Changed at 2011-03-22T05:26:43Z by arch_o_median

comment:9 Changed at 2011-05-10T19:35:03Z by david-sarah@…

In 16a2f71eea022c92:

relnotes.txt: forseeable -> foreseeable. refs #1342

comment:10 Changed at 2011-05-10T19:35:03Z by david-sarah@…

In 2dd742b24808d54a:

relnotes.txt: don't claim to work on Cygwin (which has been untested for some time). refs #1342

comment:12 in reply to: ↑ 8 Changed at 2011-05-11T14:17:11Z by davidsarah

Replying to ChosenOne:

Please review attachment:improved-package-tests.darcs.patch - I didn't fully grasp that one ;)

I saw the test-with-fake-dists.py test, that this patch is based on, looking for a package "fakedependency" on PyPI. Maybe we should split this out into a separate ticket and think more carefully about how to do it.

Last edited at 2011-05-11T14:18:07Z by davidsarah (previous) (diff)

comment:13 Changed at 2011-05-11T14:22:47Z by davidsarah

Review still needed for default-alias-cleanup.darcs.patch.

comment:15 Changed at 2011-05-11T18:51:36Z by zooko

Oh look: ChosenOne went back and edited his earlier comment to add +1's for attachment:default-alias-cleanup.darcs.patch and attachment:docs-running-no-install.darcs.patch . :-)

ChosenOne: next time please add a new comment instead of editing your old one so that people who are just checking new comments will notice. :-)

comment:16 Changed at 2011-05-12T13:40:30Z by davidsarah

Last edited at 2011-05-12T13:41:09Z by davidsarah (previous) (diff)

comment:17 Changed at 2011-05-12T13:42:46Z by david-sarah@…

In 49fd2e6e564a0e01:

scripts/common.py: don't assume that the default alias is always 'tahoe' (it is, but the API of get_alias doesn't say so). refs #1342

comment:18 Changed at 2011-06-04T01:51:15Z by zooko

  • Summary changed from post-1.8.2 clean-up to rename tests of packaging and improve them to avoid spurious system-dependent test failures

If I understand correctly, the only patch that hasn't been reviewed and committed or else superceded is attachment:improved-package-tests.darcs.patch. Changing the name of this ticket!

comment:19 Changed at 2011-07-27T18:22:12Z by zooko

  • Milestone changed from 1.9.0 to soon

comment:20 Changed at 2012-05-13T04:03:03Z by Brian Warner <warner@…>

  • Resolution set to fixed
  • Status changed from new to closed

In d86776560411accc:

modify build_helpers files

Should close #1342. This makes the actual changes to the two test
files (separated from the 'rename' patch to avoid VC complications).

comment:21 Changed at 2012-05-13T04:04:21Z by Brian Warner <warner@…>

In d86776560411accc:

modify build_helpers files

Should close #1342. This makes the actual changes to the two test
files (separated from the 'rename' patch to avoid VC complications).

comment:22 Changed at 2012-05-13T04:45:36Z by davidsarah

The original improved-package-tests.darcs.patch renamed some files, and that change hasn't been applied. However, I think the new names (e.g. misc/build_helpers/test-dont-install-newer-dep-when-you-already-have-sufficiently-new-one.py) were excessively long.

Also I don't really understand what improvement this patch is trying to make. AFAICS it hasn't been reviewed, and I was waiting for Zooko to answer comment:12.

comment:23 follow-up: Changed at 2012-05-13T04:56:05Z by warner

  • Keywords review-needed removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Actually I landed a patch just before d86776560411accc (probably 533e4bc813d299e7) that did the renames, but I forgot to include a "refs #1342", so it didn't get mentioned here. (I split the patch into two pieces, a file-move followed by file-modify, because I had to land it via git (since darcs went into an O(N!) death spiral trying to apply the dpatch) and I wanted to avoid problems in the git/darcs bridge).

I agree that the new names are excessively long, but they're in a fairly obscure corner of the tree so I decided I'd let it go. I skimmed over the changes and they seemed ok. But I did miss that you were asking zooko for something in comment:12 .. sorry about that!

In the buildbot run just now, it didn't look like it was trying to get "fakedependency" from pypi, but I might be missing something. Could you take a look at that run and see if you're still seeing the problem you observed earlier?

comment:24 Changed at 2012-05-13T04:56:19Z by warner

  • Owner changed from nobody to davidsarah
  • Status changed from reopened to new

comment:25 Changed at 2012-05-13T06:34:32Z by Brian Warner <warner@…>

In 488b6f8ccdd50eb3:

test-dont-use-too-old-dep.py: fix tarfile timestamps

It turns out that TarFile?.addfile() doesn't provide a reasonable default
timestamp, resulting in files dated to 1970 (they're probably wearing
bell-bottoms and listening to disco too). Then, when the bdist_egg command
tries to create a *zip*file with those files, it explodes because zipfiles
cannot handle timestamps before 1980 (it prefers boomboxes and jackets with
straps on the shoulders, thank you very much).

This puts a modern time.time() on the members of the tarfile, allowing future
cryptocoderarchaeologists the opportunity to make fun of fashion trends from
the user's chosen era, rather than an artificially older one.

refs #1342

comment:26 Changed at 2012-05-13T06:35:06Z by Brian Warner <warner@…>

In 488b6f8ccdd50eb3:

test-dont-use-too-old-dep.py: fix tarfile timestamps

It turns out that TarFile?.addfile() doesn't provide a reasonable default
timestamp, resulting in files dated to 1970 (they're probably wearing
bell-bottoms and listening to disco too). Then, when the bdist_egg command
tries to create a *zip*file with those files, it explodes because zipfiles
cannot handle timestamps before 1980 (it prefers boomboxes and jackets with
straps on the shoulders, thank you very much).

This puts a modern time.time() on the members of the tarfile, allowing future
cryptocoderarchaeologists the opportunity to make fun of fashion trends from
the user's chosen era, rather than an artificially older one.

refs #1342

comment:27 in reply to: ↑ 23 Changed at 2012-05-13T23:48:28Z by davidsarah

  • Status changed from new to assigned

Replying to warner:

In the buildbot run just now, it didn't look like it was trying to get "fakedependency" from pypi, but I might be missing something. Could you take a look at that run and see if you're still seeing the problem you observed earlier?

Will do.

comment:28 Changed at 2014-09-11T22:42:29Z by warner

  • Component changed from unknown to packaging
Note: See TracTickets for help on using tickets.