Closed Bug 352305 Opened 13 years ago Closed 13 years ago

Mark more clearly the parts of the xpcshell unit test example that need to be customized

Categories

(Testing :: XPCShell Harness, defect)

x86
Windows 2000
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: asqueella, Assigned: davel)

Details

Attachments

(2 files, 1 obsolete file)

See bug 352248 comments 2-4.
Attached patch like so?Splinter Review
This is as detailed as it gets without copying the docs from MDC.

I also added more assertions to the example test and enabled the test - for two reasons. First, this demonstrates the available functions and the code to add a test folder to the build system. Second, there is now a very simple test one can run when hacking the test harness itself.
Attachment #238057 - Flags: review?(davel)
Comment on attachment 238057 [details] [diff] [review]
like so?

xpcshell-simple/Makefile.in is not part of the example - it is used to install the common files.  r+ on changes to files in example directory
Attachment #238057 - Flags: review?(davel) → review+
I believe it would be better to make the "example" test an actual working test. I tried to explain why in comment 1 - it makes it a better example and it gives something very simple to test harness changes on.

I would even suggest renaming "example" folder to "test" if it wasn't such a PITA in CVS.
Could you check the patch or just its part under example/ in, please?
(In reply to comment #3)
> I believe it would be better to make the "example" test an actual working test.

Please pardon me.  I'm dense, and did not understand this before.

Now I do, and I too think it is a good idea.

I'll check in the entire patch on trunk, module tree openness.
Checked in to trunk

Checking in Makefile.in;
/cvsroot/mozilla/tools/test-harness/xpcshell-simple/Makefile.in,v  <--  Makefile.in
new revision: 1.2; previous revision: 1.1
done
Checking in example/Makefile.in;
/cvsroot/mozilla/tools/test-harness/xpcshell-simple/example/Makefile.in,v  <--  Makefile.in
new revision: 1.3; previous revision: 1.2
done
Checking in example/unit/test_sample.js;
/cvsroot/mozilla/tools/test-harness/xpcshell-simple/example/unit/test_sample.js,v  <--  test_sample.js
new revision: 1.2; previous revision: 1.1
done
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
the example test fails, because the libs:: target is not run during a normal build so some required files and directories are missing.

I believe adding the Makefile to allmakefiles.sh will fix this.  testing now, patch to follow.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Define "normal build"? libs target is built for me. allmakefiles.sh shouldn't make a difference, even though I'm not a build guru.
(In reply to comment #8)
> Define "normal build"? libs target is built for me. allmakefiles.sh shouldn't
> make a difference, even though I'm not a build guru.

make check failed on my build, from a clean checkout, using an objdir.

the libs:: target was not being run in the example dir.  it was being run in the xpcshell-simple dir.  so $(DIST)/bin/test_harness_xpcshell_simple was not being created and populated with unit tests.

I'm investigating, and am focusing on the TOOL_DIRS stuff right now instead of allmakefiles.sh.  please wait for the patch.
(In reply to comment #8)
> Define "normal build"? 

I should clarify:

cd mozilla
cvs up client.mk
make -f client.mk checkout
make -f client.mk build
I think the short-term solution is to add

ifdef ENABLE_TESTS
DIRS  += xpcshell-simple/example
endif

to tools/test-harness/Makefile.in

this will cause the libs:: target in tools/test-harness/xpcshell-simple/example to be invoked.

The lines in tools/test-harness-xpcshell-simple/Makefile.in:

ifdef ENABLE_TESTS
TOOL_DIRS  += example
endif

have no effect since the tools target is not run as part of tier_testharness.

Changing DIRS to TOOL_DIRS in the change to tools/test-harness/Makefile.in fails to fix this problem, for the same reason as above.

Changing the build system to invoke tools_tier_testharness is probably a better fix.

Figuring out what is the right way for the build system to deal with this case, then changing the build system and this case accordingly, is probably an even better fix.

I'm going to test the workaround (add example to DIRS in tools/test-harness/Makefile.in) then attach a patch if it works.

I'll open a bug against the build system to track figuring out what changes should be made there.
Attached patch short-term workaround (obsolete) — Splinter Review
asqueella, could you please try this on your machine to double-check the fix?
Attachment #239932 - Flags: review?
Comment on attachment 239932 [details] [diff] [review]
short-term workaround

I just noticed that the example unit test is now run twice by make check.  I feel this is ok for this short-term workaround
Yep, this makes the test files copied for me during a normal build. As I mentioned on IRC, it worked for me because I just did 'make' in test-harness, which does run 'make tools', and never made a clean build.

Knowing as little about our system as I do, I wonder for what modules is 'make tools' not run and why. Should http://developer.mozilla.org/en/docs/Writing_xpcshell-based_unit_tests#Creating_the_first_test_for_a_module be revised?
asqueella, can you try this one out and let me know if it works for you?
Attachment #239932 - Attachment is obsolete: true
Attachment #239932 - Flags: review?
Sorry for the delay. It appears to work, but I don't understand, why was TOOL_DIRS used initially? Did you run this by bsmedberg?

If it's the final fix, the docs need to be updated.
(In reply to comment #16)
> why was TOOL_DIRS used initially?

Historical reasons.  it happens to work for necko.

> Did you run this by bsmedberg?

yes.  there's more to work out.

> If it's the final fix, the docs need to be updated.

yep. I'll take care of that when I check it in.
Checking in Makefile.in;
/cvsroot/mozilla/tools/test-harness/xpcshell-simple/Makefile.in,v  <--  Makefile.in
new revision: 1.3; previous revision: 1.2
done

(also updating docs on mdc)
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Assignee: nobody → mozmail
Component: Testing → TUnit
Product: Core → Testing
QA Contact: testing → tunit
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.