You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Stanton Sievers <si...@gmail.com> on 2012/05/14 16:15:00 UTC

Review Request: Closure compile all feature JavaScript as part of JUnit execution to ensure no JavaScript errors at build time

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5107/
-----------------------------------------------------------

Review request for shindig.


Summary
-------

See the JIRA for the high-level description.

My approach to doing this was to introduce a new test case in the end-to-end tests.  This way I can test with a real FeatureRegistry against the real web app.  I make a request to the JsServlet for all of the features, which will run through the compiler processors.  Because of the changes that went in for https://issues.apache.org/jira/browse/SHINDIG-1770, we will return the compile error and this will get printed as part of the JUnit assert failure, pin-pointing the exact compile problem.

Note that this does increase the time it takes to execute the EndToEndTest on the scale of tens of seconds.


This addresses bug SHINDIG-1771.
    https://issues.apache.org/jira/browse/SHINDIG-1771


Diffs
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/AllJsFilter.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java 1337280 
  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1337280 

Diff: https://reviews.apache.org/r/5107/diff


Testing
-------

Introduced some JS errors and verified that the unit test failed.  Additionally verified the reason for the failure was readily available.


Thanks,

Stanton


Re: Review Request: Closure compile all feature JavaScript as part of JUnit execution to ensure no JavaScript errors at build time

Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5107/#review7883
-----------------------------------------------------------

Ship it!


Committed revision 1338499.

- Stanton


On 2012-05-15 01:46:03, Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5107/
> -----------------------------------------------------------
> 
> (Updated 2012-05-15 01:46:03)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> See the JIRA for the high-level description.
> 
> My approach to doing this was to introduce a new test case in the end-to-end tests.  This way I can test with a real FeatureRegistry against the real web app.  I make a request to the JsServlet for all of the features, which will run through the compiler processors.  Because of the changes that went in for https://issues.apache.org/jira/browse/SHINDIG-1770, we will return the compile error and this will get printed as part of the JUnit assert failure, pin-pointing the exact compile problem.
> 
> Note that this does increase the time it takes to execute the EndToEndTest on the scale of tens of seconds.
> 
> 
> This addresses bug SHINDIG-1771.
>     https://issues.apache.org/jira/browse/SHINDIG-1771
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/AllJsFilter.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java 1337280 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1337280 
> 
> Diff: https://reviews.apache.org/r/5107/diff
> 
> 
> Testing
> -------
> 
> Introduced some JS errors and verified that the unit test failed.  Additionally verified the reason for the failure was readily available.
> 
> 
> Thanks,
> 
> Stanton
> 
>


Re: Review Request: Closure compile all feature JavaScript as part of JUnit execution to ensure no JavaScript errors at build time

Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5107/
-----------------------------------------------------------

(Updated 2012-05-15 01:46:03.019915)


Review request for shindig.


Changes
-------

Making the all feature name a little more unique so as to avoid any collisions.


Summary
-------

See the JIRA for the high-level description.

My approach to doing this was to introduce a new test case in the end-to-end tests.  This way I can test with a real FeatureRegistry against the real web app.  I make a request to the JsServlet for all of the features, which will run through the compiler processors.  Because of the changes that went in for https://issues.apache.org/jira/browse/SHINDIG-1770, we will return the compile error and this will get printed as part of the JUnit assert failure, pin-pointing the exact compile problem.

Note that this does increase the time it takes to execute the EndToEndTest on the scale of tens of seconds.


This addresses bug SHINDIG-1771.
    https://issues.apache.org/jira/browse/SHINDIG-1771


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/AllJsFilter.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java 1337280 
  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1337280 

Diff: https://reviews.apache.org/r/5107/diff


Testing
-------

Introduced some JS errors and verified that the unit test failed.  Additionally verified the reason for the failure was readily available.


Thanks,

Stanton


Re: Review Request: Closure compile all feature JavaScript as part of JUnit execution to ensure no JavaScript errors at build time

Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5107/#review7852
-----------------------------------------------------------

Ship it!


this is awesome! +1

- Henry


On 2012-05-14 17:00:10, Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5107/
> -----------------------------------------------------------
> 
> (Updated 2012-05-14 17:00:10)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> See the JIRA for the high-level description.
> 
> My approach to doing this was to introduce a new test case in the end-to-end tests.  This way I can test with a real FeatureRegistry against the real web app.  I make a request to the JsServlet for all of the features, which will run through the compiler processors.  Because of the changes that went in for https://issues.apache.org/jira/browse/SHINDIG-1770, we will return the compile error and this will get printed as part of the JUnit assert failure, pin-pointing the exact compile problem.
> 
> Note that this does increase the time it takes to execute the EndToEndTest on the scale of tens of seconds.
> 
> 
> This addresses bug SHINDIG-1771.
>     https://issues.apache.org/jira/browse/SHINDIG-1771
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/AllJsFilter.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java 1337280 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1337280 
> 
> Diff: https://reviews.apache.org/r/5107/diff
> 
> 
> Testing
> -------
> 
> Introduced some JS errors and verified that the unit test failed.  Additionally verified the reason for the failure was readily available.
> 
> 
> Thanks,
> 
> Stanton
> 
>


Re: Review Request: Closure compile all feature JavaScript as part of JUnit execution to ensure no JavaScript errors at build time

Posted by Stanton Sievers <si...@gmail.com>.

> On 2012-05-14 20:55:46, Matt Franklin wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/AllJsFilter.java, line 65
> > <https://reviews.apache.org/r/5107/diff/3/?file=108821#file108821line65>
> >
> >     You might want to consider adding something to the documentation stating that 'all' is a reserved feature name.   Not that it is likely, but there could be a collision.

Keep in mind this filter only exists for the duration of the EndToEndTests.  Is your concern that a real "all" feature could exist and we would invalidate any testing against it?  I'm not sure where I'd document it either.  Suggestions?


- Stanton


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5107/#review7867
-----------------------------------------------------------


On 2012-05-14 17:00:10, Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5107/
> -----------------------------------------------------------
> 
> (Updated 2012-05-14 17:00:10)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> See the JIRA for the high-level description.
> 
> My approach to doing this was to introduce a new test case in the end-to-end tests.  This way I can test with a real FeatureRegistry against the real web app.  I make a request to the JsServlet for all of the features, which will run through the compiler processors.  Because of the changes that went in for https://issues.apache.org/jira/browse/SHINDIG-1770, we will return the compile error and this will get printed as part of the JUnit assert failure, pin-pointing the exact compile problem.
> 
> Note that this does increase the time it takes to execute the EndToEndTest on the scale of tens of seconds.
> 
> 
> This addresses bug SHINDIG-1771.
>     https://issues.apache.org/jira/browse/SHINDIG-1771
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/AllJsFilter.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java 1337280 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1337280 
> 
> Diff: https://reviews.apache.org/r/5107/diff
> 
> 
> Testing
> -------
> 
> Introduced some JS errors and verified that the unit test failed.  Additionally verified the reason for the failure was readily available.
> 
> 
> Thanks,
> 
> Stanton
> 
>


Re: Review Request: Closure compile all feature JavaScript as part of JUnit execution to ensure no JavaScript errors at build time

Posted by Matt Franklin <mf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5107/#review7867
-----------------------------------------------------------

Ship it!


LGTM


http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/AllJsFilter.java
<https://reviews.apache.org/r/5107/#comment17198>

    You might want to consider adding something to the documentation stating that 'all' is a reserved feature name.   Not that it is likely, but there could be a collision.


- Matt


On 2012-05-14 17:00:10, Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5107/
> -----------------------------------------------------------
> 
> (Updated 2012-05-14 17:00:10)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> See the JIRA for the high-level description.
> 
> My approach to doing this was to introduce a new test case in the end-to-end tests.  This way I can test with a real FeatureRegistry against the real web app.  I make a request to the JsServlet for all of the features, which will run through the compiler processors.  Because of the changes that went in for https://issues.apache.org/jira/browse/SHINDIG-1770, we will return the compile error and this will get printed as part of the JUnit assert failure, pin-pointing the exact compile problem.
> 
> Note that this does increase the time it takes to execute the EndToEndTest on the scale of tens of seconds.
> 
> 
> This addresses bug SHINDIG-1771.
>     https://issues.apache.org/jira/browse/SHINDIG-1771
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/AllJsFilter.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java 1337280 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1337280 
> 
> Diff: https://reviews.apache.org/r/5107/diff
> 
> 
> Testing
> -------
> 
> Introduced some JS errors and verified that the unit test failed.  Additionally verified the reason for the failure was readily available.
> 
> 
> Thanks,
> 
> Stanton
> 
>


Re: Review Request: Closure compile all feature JavaScript as part of JUnit execution to ensure no JavaScript errors at build time

Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5107/#review7868
-----------------------------------------------------------

Ship it!


LGTM

- Ryan


On 2012-05-14 17:00:10, Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5107/
> -----------------------------------------------------------
> 
> (Updated 2012-05-14 17:00:10)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> See the JIRA for the high-level description.
> 
> My approach to doing this was to introduce a new test case in the end-to-end tests.  This way I can test with a real FeatureRegistry against the real web app.  I make a request to the JsServlet for all of the features, which will run through the compiler processors.  Because of the changes that went in for https://issues.apache.org/jira/browse/SHINDIG-1770, we will return the compile error and this will get printed as part of the JUnit assert failure, pin-pointing the exact compile problem.
> 
> Note that this does increase the time it takes to execute the EndToEndTest on the scale of tens of seconds.
> 
> 
> This addresses bug SHINDIG-1771.
>     https://issues.apache.org/jira/browse/SHINDIG-1771
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/AllJsFilter.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java 1337280 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1337280 
> 
> Diff: https://reviews.apache.org/r/5107/diff
> 
> 
> Testing
> -------
> 
> Introduced some JS errors and verified that the unit test failed.  Additionally verified the reason for the failure was readily available.
> 
> 
> Thanks,
> 
> Stanton
> 
>


Re: Review Request: Closure compile all feature JavaScript as part of JUnit execution to ensure no JavaScript errors at build time

Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5107/
-----------------------------------------------------------

(Updated 2012-05-14 17:00:10.525787)


Review request for shindig.


Changes
-------

Minor update on code style.


Summary
-------

See the JIRA for the high-level description.

My approach to doing this was to introduce a new test case in the end-to-end tests.  This way I can test with a real FeatureRegistry against the real web app.  I make a request to the JsServlet for all of the features, which will run through the compiler processors.  Because of the changes that went in for https://issues.apache.org/jira/browse/SHINDIG-1770, we will return the compile error and this will get printed as part of the JUnit assert failure, pin-pointing the exact compile problem.

Note that this does increase the time it takes to execute the EndToEndTest on the scale of tens of seconds.


This addresses bug SHINDIG-1771.
    https://issues.apache.org/jira/browse/SHINDIG-1771


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/AllJsFilter.java PRE-CREATION 
  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java 1337280 
  http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1337280 

Diff: https://reviews.apache.org/r/5107/diff


Testing
-------

Introduced some JS errors and verified that the unit test failed.  Additionally verified the reason for the failure was readily available.


Thanks,

Stanton


Re: Review Request: Closure compile all feature JavaScript as part of JUnit execution to ensure no JavaScript errors at build time

Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5107/#review7842
-----------------------------------------------------------

Ship it!


LGTM

- Dan


On 2012-05-14 14:15:00, Stanton Sievers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5107/
> -----------------------------------------------------------
> 
> (Updated 2012-05-14 14:15:00)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> See the JIRA for the high-level description.
> 
> My approach to doing this was to introduce a new test case in the end-to-end tests.  This way I can test with a real FeatureRegistry against the real web app.  I make a request to the JsServlet for all of the features, which will run through the compiler processors.  Because of the changes that went in for https://issues.apache.org/jira/browse/SHINDIG-1770, we will return the compile error and this will get printed as part of the JUnit assert failure, pin-pointing the exact compile problem.
> 
> Note that this does increase the time it takes to execute the EndToEndTest on the scale of tens of seconds.
> 
> 
> This addresses bug SHINDIG-1771.
>     https://issues.apache.org/jira/browse/SHINDIG-1771
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/AllJsFilter.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndServer.java 1337280 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/server/src/test/java/org/apache/shindig/server/endtoend/EndToEndTest.java 1337280 
> 
> Diff: https://reviews.apache.org/r/5107/diff
> 
> 
> Testing
> -------
> 
> Introduced some JS errors and verified that the unit test failed.  Additionally verified the reason for the failure was readily available.
> 
> 
> Thanks,
> 
> Stanton
> 
>