I build with --disable-tests but embedding/browser/gtk/tests still gets built. Can/should this get turned off if we're configured with --disable-tests?
Probably. There's probably just a test missing around the directory in the makefile.
We are going to build this even when tests are disabled at leaf's request. They need it for smoketesting.
If a test is needed for a testing purpose, shouldn't that build be done WITHOUT --disable-tests?
we switched from gtkEmbed to TestGtkEmbed as our default test embed app, I wanted this built by default so we don't regress there. I believe that is why we changed the tests directory to be built by default. Perhaps we could rearrange directories to satisfy everyone? Adding leaf who r='d this change earlier this summer.
colin, the problem is that we have different levels of testing; we don't want to have to build *every* test in the release builds (increasing build time/package size) just to get one or two really high level testing applications. mcafee, changing directories is probably a good idea, but the objection is still the same... it's a testing thing that a lot of people don't care about. Really, we should break down the kinds of tests that we can build, and have flags that can override the --disable-tests option in order to build specific test apps (--enable-embedding-tests ?).
*** Bug 128583 has been marked as a duplicate of this bug. ***
> --enable-embedding-tests Yes, I think this is better. If I say "no tests", then give me no tests, please :-). REOPENing. This might be important, if a distributor doesn't use the packages-<platform> files, but just zipps up dist/bin. TestGtkEmbed shoudn't ship to users. (Beonex currently does use packages-<platform> files, so we are not affected by this bug other than during testing.)
Created attachment 193727 [details] [diff] [review] new patch This patch implements the proposed behaviour.
Comment on attachment 193727 [details] [diff] [review] new patch You don't need to include configure in the patch... our CVS system auto-commits configure whenever configure.in is changed. in embedding/browser/gtk/Makefile.in you don't need all the DO_TESTS stuff: if you really need to check ENABLE_TESTS *and* ENABLE_EMBEDDING_TESTS you can do ifneq (,$(ENABLE_TESTS)$(ENABLE_EMBEDDING_TESTS)) but I think that you can safely just do ifdef ENABLE_EMBEDDING_TESTS here, right? There are situations where people might want to --enable-tests --disable-embedding-tests
Created attachment 193933 [details] [diff] [review] New patch You're right. Here is the modified patch.
Comment on attachment 193933 [details] [diff] [review] New patch You don't need CPPSRCS = $(NULL), just remove that line altogether.... actually, why do you need the change in embedding/browser/gtk/tests/Makefile.in at all? We're not going to build that directory unless ENABLE_EMBEDDING_TESTS is set, right?
Comment on attachment 193933 [details] [diff] [review] New patch clearing review based on the question about embedding/browser/gtk/tests/Makefile.in -- please re-request review if this is somehow correct.
Created attachment 194287 [details] [diff] [review] Updated patch You're right. Simplified patch attached.
could this be landed somehow ?
Mike, if you wouldn't mind making sure this patch still works against CVS, I can help you get it checked into the tree (once it has approval).
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=Makefile.in&branch=1.13&root=/cvsroot&subdir=mozilla/embedding/browser/gtk&command=DIFF_FRAMESET&rev1=1.8&rev2=1.9 This change made the test not building when disabling tests, but there is no bug reference to know why this has been done, especially not even notifying the current bug, considering concerns had been raised about necessity of the test. The strange thing is that embedding/browser/gtk/gtkembed.pkg has not been changed, I don't know if it implies some breakage when --disable-tests is used (not tested). So now the question is, is the --enable-embedding-tests still needed ?
bsmedberg/ted, think one of you could answer Mike's question in comment #17?
Let's just assume the answer to comment #17 is "no".