Closed Bug 446300 Opened 16 years ago Closed 15 years ago

move tools/test-harness/xpcshell-simple to testing/xpcshell

Categories

(Testing :: XPCShell Harness, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: ted, Assigned: sgautherie)

References

Details

(Keywords: fixed1.9.1, Whiteboard: [fixed1.9.1b3])

Attachments

(4 files, 4 obsolete files)

I think we have a top-level testing dir, and ought to keep testsuite stuff there.
Version: unspecified → Trunk
Let's do this before bug 469523 which will probably add/rewrite things in Python...
Blocks: 469523
Sounds good, yeah.
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090115 Minefield/3.2a1pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/6eb627215b61 + bug 446300 patch)

I (clobber) built and ran |make check| successfully.
Attachment #357164 - Flags: review?(ted.mielczarek)
Attachment #357164 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 357164 [details] [diff] [review]
(Av1-MC) Move the source harness directory

rename to testing/Makefile.in

Don't bother putting a makefile in testing. Makefiles with nothing but DIRS are a waste of space (and an extra invocation of make, to boot). Just make tier_testharness_dirs = testing/xpcshell directly.

Otherwise looks good, th anks for taking this.
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090117 Minefield/3.2a1pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/184eadf4e185 + bug 446300 patch)

Av1-MC, with comment 5 suggestion(s).
Attachment #357164 - Attachment is obsolete: true
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090117 SeaMonkey/2.0a3pre] (experimental/_m-c_, home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/a3abe1807f71 + bug 446300 patch
 +http://hg.mozilla.org/comm-central/rev/da4ab57196d6 + bug 446300 patch)

I (clobber) built and ran |make check| successfully.
Attachment #357505 - Flags: review?(kairo)
Attachment #357505 - Flags: review?(kairo) → review?(bugzilla)
Attachment #357505 - Flags: review?(bugzilla) → review+
Comment on attachment 357505 [details] [diff] [review]
(Bv1-CC) Move the source harness directory
[Checkin: Comment 11]

+testxpcdir = $(MOZILLA_SRCDIR)/testing/xpcshell

There's not really much point in this variable. Its unlikely that any further updates would miss changes if they didn't have it.

However please keep it in the same style as the mozilla/ version.

r=me assuming that and you're going to land these on m-c, m-1.9.1 and c-c at pretty much the same time.
Comment on attachment 357486 [details] [diff] [review]
(Av1a-MC) Move the source harness directory
[Checkin: Comment 9+12+13]


http://hg.mozilla.org/mozilla-central/rev/42c8844fe563
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e8aaec14e0a4
Attachment #357486 - Attachment description: (Av1a-MC) Move the source harness directory → (Av1a-MC) Move the source harness directory [Checkin: Comment 9]
(In reply to comment #8)
> (From update of attachment 357505 [details] [diff] [review])
> +testxpcdir = $(MOZILLA_SRCDIR)/testing/xpcshell
> 
> There's not really much point in this variable. Its unlikely that any further
> updates would miss changes if they didn't have it.
> 
> However please keep it in the same style as the mozilla/ version.

(for now) I left it, in both places...
Comment on attachment 357505 [details] [diff] [review]
(Bv1-CC) Move the source harness directory
[Checkin: Comment 11]

(Ah, hit Enter to soon.)

http://hg.mozilla.org/comm-central/rev/f387942413e2
Attachment #357505 - Attachment description: (Bv1-CC) Move the source harness directory → (Bv1-CC) Move the source harness directory [Checkin: Comment 11]
Comment on attachment 357486 [details] [diff] [review]
(Av1a-MC) Move the source harness directory
[Checkin: Comment 9+12+13]


http://hg.mozilla.org/mozilla-central/rev/2cff77c405f8
(Cv1-MC) Move the source harness directory, followup
Move the 2 new files from bug 470914 too
Attachment #357486 - Attachment description: (Av1a-MC) Move the source harness directory [Checkin: Comment 9] → (Av1a-MC) Move the source harness directory [Checkin: Comment 9+12]
Flags: in-testsuite-
Keywords: fixed1.9.1
Whiteboard: [ToDo: comment 3]
Target Milestone: mozilla1.9.1b3 → mozilla1.9.2a1
Comment on attachment 357486 [details] [diff] [review]
(Av1a-MC) Move the source harness directory
[Checkin: Comment 9+12+13]


(In reply to comment #12)

http://hg.mozilla.org/mozilla-central/rev/ebde0d5039ee
(Dv1-MC) Move the source harness directory, followup
Fix changeset 2cff77c405f8: move new files (and update them) to the correct place.
Attachment #357486 - Attachment description: (Av1a-MC) Move the source harness directory [Checkin: Comment 9+12] → (Av1a-MC) Move the source harness directory [Checkin: Comment 9+12+13]
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090127 Minefield/3.2a1pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/13e95c7ff78b + bug 446300 patch)

I (clobber) built and ran |make check| successfully.
Attachment #359089 - Flags: review?(ted.mielczarek)
(untested, but trivial) copy/port of Ev1-MC patch.
Attachment #359090 - Flags: review?(bugzilla)
(the right patch :->)
Attachment #359090 - Attachment is obsolete: true
Attachment #359092 - Flags: review?(bugzilla)
Attachment #359090 - Flags: review?(bugzilla)
Comment on attachment 359089 [details] [diff] [review]
(Ev1-MC) Rename the object tests directory

 var gTestRoot = dirSvc.get("CurProcD", Ci.nsILocalFile);


Why not change this to use gTestRoot = __LOCATION__.parent now that we've got that? (That will make it a lot more future-proof!)

The build config parts of the patch are fine, though.
Attachment #359089 - Flags: review?(ted.mielczarek) → review-
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090127 Minefield/3.2a1pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/13e95c7ff78b + bug 446300 patch)

Ev1-MC, with comment 17 suggestion(s).

***

While there, I'm also wondering about the following paths:
source\mozilla\testing\mochitest
source\mozilla\testing\xpcshell
objdir\_tests\testing\mochitest
objdir\_tests\xpcshell

If we want all of them to be similar, we should use 'objdir\_tests\testing\xpcshell'. (this patch)
If we don't care about the "empty" 'testing' dir, we should (move/)use 'objdir\_tests\mochitest'. (separate patch)
Attachment #359089 - Attachment is obsolete: true
Attachment #359388 - Flags: review?(ted.mielczarek)
(In reply to comment #18)
> If we want all of them to be similar, we should use
> 'objdir\_tests\testing\xpcshell'. (this patch)
> If we don't care about the "empty" 'testing' dir, we should (move/)use
> 'objdir\_tests\mochitest'. (separate patch)

I don't know what ted thinks, but I would prefer them not to be in the testing/ directory. For example to get to an xpcshell log you currently have to go into 4 directories deep (which happens to be 5 on comm-central), but with this patch we'd have to do 5 directories which seems a little excessive.
Yeah, I'd prefer to get rid of the "testing". I think mochitest is in there just to parallel the source directory structure, but I don't think that's really necessary. "_tests/xpcshell" seems fine, and we could move mochitest to "_tests/mochitest", although you'll have to let people know, because people tend to run runtests.py directly.
Comment on attachment 359092 [details] [diff] [review]
(Fv1-CC) Rename the object tests directory

There's certainly one if not two parts of this patch that have been bitrotted, though I think the general idea is fine.
Attachment #359092 - Flags: review?(bugzilla)
Attachment #359388 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 359388 [details] [diff] [review]
(Ev2-MC) Rename the object tests directory
[Checkin: See comment 29 & 30]

Looks good, thanks.
Fv1-CC, with comment 21 suggestion(s).
Attachment #359092 - Attachment is obsolete: true
Attachment #360175 - Flags: review?(bugzilla)
Comment on attachment 360175 [details] [diff] [review]
(Fv1a-CC) Rename the object tests directory
[Checkin: Comment 28]

Whilst this looks reasonable, applying this patch and the m-c patch, and doing a clobber build of Thunderbird with no debug but enable tests I get:

TEST-UNEXPECTED-FAIL | ../../../_tests/xpcshell/test_plugin/unit/test_bug455213.js | test failed, see log
../../../_tests/xpcshell/test_plugin/unit/test_bug455213.js.log:
>>>>>>>
*** test pending
ReferenceError: __LOCATION__ is not defined
*** FAIL ***

<<<<<<<
Attachment #360175 - Flags: review?(bugzilla) → review-
bug 470914 hasn't landed on 1.9.1. It could, there's no real reason not to.
Depends on: 470914
Attachment #360175 - Flags: review- → review?(bugzilla)
Comment on attachment 360175 [details] [diff] [review]
(Fv1a-CC) Rename the object tests directory
[Checkin: Comment 28]

(In reply to comment #24)
> Whilst this looks reasonable, applying this patch and the m-c patch, and doing
> a clobber build of Thunderbird with no debug but enable tests I get:

I assume you tested a TBv3.0/m-1.9.1 build.
If so, I just landed bug 470914 on 1.9.1 so it should work now.
Comment on attachment 360175 [details] [diff] [review]
(Fv1a-CC) Rename the object tests directory
[Checkin: Comment 28]

Yep, that's better. r=me as long as you land mozilla-1.9.1 and comm-central versions at the same time (and preferably mozilla-central not too far away in time as well).
Attachment #360175 - Flags: review?(bugzilla) → review+
Comment on attachment 360175 [details] [diff] [review]
(Fv1a-CC) Rename the object tests directory
[Checkin: Comment 28]


http://hg.mozilla.org/comm-central/rev/bcad9ca1e2b6
Attachment #360175 - Attachment description: (Fv1a-CC) Rename the object tests directory → (Fv1a-CC) Rename the object tests directory [Checkin: Comment 28]
Comment on attachment 359388 [details] [diff] [review]
(Ev2-MC) Rename the object tests directory
[Checkin: See comment 29 & 30]


http://hg.mozilla.org/mozilla-central/rev/9f4b91af7e5f

after fixing context for
{
patching file toolkit/mozapps/extensions/test/Makefile.in
Hunk #1 FAILED at 34
1 out of 1 hunks FAILED
patching file toolkit/mozapps/update/test/Makefile.in
Hunk #1 FAILED at 42
1 out of 1 hunks FAILED
}
Attachment #359388 - Attachment description: (Ev2-MC) Rename the object tests directory → (Ev2-MC) Rename the object tests directory [Checkin: See comment 29 & 30]
Comment on attachment 359388 [details] [diff] [review]
(Ev2-MC) Rename the object tests directory
[Checkin: See comment 29 & 30]


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/426d8b8b6f24

after fixing context for
{
patching file toolkit/mozapps/update/test/Makefile.in
Hunk #1 FAILED at 42
1 out of 1 hunks FAILED
}
(In reply to comment #20)
> move mochitest to "_tests/mochitest"

I filed bug 479998.
Severity: normal → minor
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [ToDo: comment 3] → [fixed1.9.1b3]
You need to log in before you can comment on or make changes to this bug.