Opened at 2011-01-28T15:03:16Z
Last modified at 2014-09-11T22:42:29Z
#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)
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-30T15:24:33Z by zooko
comment:2 Changed at 2011-01-30T15:26:21Z by zooko
please review: attachment:improved-package-tests.darcs.patch
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:3 Changed at 2011-01-30T17:33:45Z by davidsarah
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:7 Changed at 2011-03-23T23:35:15Z by zooko
please review: attachment:docfix.darcs.patch
comment:8 follow-up: ↓ 12 Changed at 2011-05-10T19:29:30Z by ChosenOne
+1 on attachment:relnotes.darcs.patch,
attachment:docfix.darcs.patch, attachment:tighter-catch.darcs.patch, attachment:docs-running-no-install.darcs.patch and attachment:default-alias-cleanup.darcs.patch
Please review attachment:improved-package-tests.darcs.patch - I didn't fully grasp that one ;)
comment:9 Changed at 2011-05-10T19:35:03Z by david-sarah@…
In 16a2f71eea022c92:
comment:10 Changed at 2011-05-10T19:35:03Z by david-sarah@…
In 2dd742b24808d54a:
comment:11 Changed at 2011-05-10T19:42:52Z by zooko
- applied attachment:tighter-catch.darcs.patch in b30a269ec2477e84
- applied attachment:docfix.darcs.patch in 9619812a9db74aab
- applied attachment:relnotes.darcs.patch in 2dd742b24808d54a and 16a2f71eea022c92
- added a M-x whitespace-cleanup in f251bbece5f8f188
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.
comment:13 Changed at 2011-05-11T14:22:47Z by davidsarah
Review still needed for default-alias-cleanup.darcs.patch.
comment:14 Changed at 2011-05-11T18:47:16Z by zooko
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
docs-running-no-install.darcs.patch is obsoleted by 299e8ad5795d3c22 (which deletes docs/running.html and changes docs/running.rst to point to docs/quickstart.rst).
comment:17 Changed at 2011-05-12T13:42:46Z by david-sarah@…
In 49fd2e6e564a0e01:
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:
comment:21 Changed at 2012-05-13T04:04:21Z by Brian Warner <warner@…>
In d86776560411accc:
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: ↓ 27 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:
comment:26 Changed at 2012-05-13T06:35:06Z by Brian Warner <warner@…>
In 488b6f8ccdd50eb3:
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
please review: attachment:tighter-catch.darcs.patch