Last Comment Bug 755080 - Add some tests for binding codegen
: Add some tests for binding codegen
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: ParisBindings 753517 755636
  Show dependency treegraph
 
Reported: 2012-05-14 15:37 PDT by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2012-05-24 10:51 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
part 1. Add infrastructure for binding codegen tests. (12.99 KB, patch)
2012-05-15 17:59 PDT, Boris Zbarsky [:bz] (still a bit busy)
no flags Details | Diff | Splinter Review
part 2. Exercise integer types a bit. (6.71 KB, patch)
2012-05-15 19:23 PDT, Boris Zbarsky [:bz] (still a bit busy)
peterv: review+
Details | Diff | Splinter Review
part 3. Exercise castable interface types a bit. (4.23 KB, patch)
2012-05-15 23:22 PDT, Boris Zbarsky [:bz] (still a bit busy)
no flags Details | Diff | Splinter Review
part 3. Exercise castable interface types a bit. (4.55 KB, patch)
2012-05-15 23:25 PDT, Boris Zbarsky [:bz] (still a bit busy)
peterv: review+
Details | Diff | Splinter Review
Part 1 disentangled from the CSS declaration stuff (13.93 KB, patch)
2012-05-18 12:44 PDT, Boris Zbarsky [:bz] (still a bit busy)
khuey: review+
peterv: review+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2012-05-14 15:37:17 PDT
Need to at least:

1)  Have some test IDL.
2)  Make sure it gets parsed by the IDL parser.
3)  Make sure we can generate code from it.

and preferably:

4)  Make sure the code compiles against some C++ headers (and that the headers
    try to check that the code is somewhat correct in terms of the types it
    uses).

and ideally:

5)  Make sure the runtime behavior is correct.

I'll try to do at least 1-3, maybe #4 here; won't worry about #5 too much so far.
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2012-05-15 17:59:36 PDT
Created attachment 624263 [details] [diff] [review]
part 1.  Add infrastructure for binding codegen tests.
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2012-05-15 19:23:15 PDT
Created attachment 624278 [details] [diff] [review]
part 2.  Exercise integer types a bit.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2012-05-15 23:22:32 PDT
Created attachment 624305 [details] [diff] [review]
part 3.  Exercise castable interface types a bit.

I'll add some sequence tests in the bug where I convert those to dom::Sequence.  I'll also add some tests of the infallible stuff in the bug on moving that annotation to the IDL.  In general, people should feel free to add more stuff here!
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2012-05-15 23:25:26 PDT
Created attachment 624306 [details] [diff] [review]
part 3.  Exercise castable interface types a bit.
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2012-05-15 23:27:14 PDT
One more note.  I don't know exactly how to write tests for something like bug 754457...  If anyone has bright ideas, let me know.
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2012-05-18 12:44:57 PDT
Created attachment 625208 [details] [diff] [review]
Part 1 disentangled from the CSS declaration stuff
Comment 7 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-05-21 11:23:22 PDT
Comment on attachment 625208 [details] [diff] [review]
Part 1 disentangled from the CSS declaration stuff

Review of attachment 625208 [details] [diff] [review]:
-----------------------------------------------------------------

Yuck, but ok.

::: dom/Makefile.in
@@ +106,5 @@
>    $(NULL)
>  endif
>  
> +# bindings/test is here, because it needs to build after bindings, and
> +# we build kids before ourselves.

s/kids/subdirectories/ please.

Also, please say 'bindings/' instead of 'bindings' so it's clear we're referring to a directory

::: dom/bindings/Makefile.in
@@ +39,5 @@
> +ifdef ENABLE_TESTS
> +test_webidl_files := TestCodeGen.webidl
> +else
> +test_webidl_files := $(NULL)
> +endif

Can we stick test_webidl_files in WebIDL.mk please?
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2012-05-21 11:41:59 PDT
Made the changes comment 7 asks for.  In dom/bindings/test/Makefile.in, I now do this:

  # And need this for $(test_webidl_files)
  include $(topsrcdir)/dom/webidl/WebIDL.mk

  # But the webidl actually lives in our parent dir
  test_webidl_files := $(addprefix ../,$(test_webidl_files))

to make the paths work out.
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2012-05-23 09:49:15 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/24f6cf7eece9
https://hg.mozilla.org/integration/mozilla-inbound/rev/13e1c5f61e38
https://hg.mozilla.org/integration/mozilla-inbound/rev/afb26ea68c7f

Please pretty please everyone go and add tests!  But after I manage to check in bug 743906, so we don't conflict up the wazoo...
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2012-05-24 10:51:32 PDT
Nah, this is fixed.  More work should happen in separate bugs.

Note You need to log in before you can comment on or make changes to this bug.