will create RFC in Wave soon.
http://gwt-code-reviews.appspot.com/1043801/diff/1/4
File dev/core/src/com/google/gwt/dev/cfg/SyntheticModuleDef.java
(right):
http://gwt-code-reviews.appspot.com/1043801/diff/1/4#newcode50
dev/core/src/com/google/gwt/dev/cfg/SyntheticModuleDef.java:50: public
synchronized String getPrimaryModuleName() {
I think the refactoring has a positive effect even if we didn't need to
add this method.
The current code was a hack IMHO. This refactoring makes it clear that
synthetic modules are subclass with different behaviors. Since all
changed references (all under junit) actually expect a synthetic module
instead of a plain module, now the contract is enforced by static
typing.
On 2010/11/09 02:51:11, jat wrote:
I think rather than changing all the references of ModuleDef to
SyntheticModuleDef, you should just add this functionality into
ModuleDef.
http://gwt-code-reviews.appspot.com/1043801/diff/1/5
File tools/api-checker/config/gwt20_21userApi.conf (right):
http://gwt-code-reviews.appspot.com/1043801/diff/1/5#newcode73
tools/api-checker/config/gwt20_21userApi.conf:73:
:user/src/com/google/gwt/junit/client/TestServiceFactory.java\
I'm not sure about the "official" way to add APIs. I did this because
GWTTestCase is also excluded.
On 2010/11/09 02:51:11, jat wrote:
Why are these excluded? If they aren't in impl packages, we are
saying we will
support the API.
http://gwt-code-reviews.appspot.com/1043801/diff/1/5#newcode148
tools/api-checker/config/gwt20_21userApi.conf:148:
com.google.gwt.junit.client.impl.GWTRunner MISSING
Again, I need decisions by veterans of GWT API changes.
On 2010/11/09 02:51:11, jat wrote:
Maybe all classes in impl packages should be excluded (around line 87)
instead.
http://gwt-code-reviews.appspot.com/1043801/diff/1/7
File user/src/com/google/gwt/junit/E2eJUnitShell.java (right):
http://gwt-code-reviews.appspot.com/1043801/diff/1/7#newcode80
user/src/com/google/gwt/junit/E2eJUnitShell.java:80:
options.setProxyToHost(getHost());
super.maybeStartProxy() will start the proxy filter if the '-proxyTo'
option is set. This is an easy way to reuse code.
I can also reuse the code by creating
protected final void startProxy()
and call it conditionally in GWTTestCase and always call it here. Does
this sound better?
On 2010/11/09 02:51:11, jat wrote:
Why do we need this?
http://gwt-code-reviews.appspot.com/1043801/diff/1/8
File user/src/com/google/gwt/junit/JUnitShell.java (right):
http://gwt-code-reviews.appspot.com/1043801/diff/1/8#newcode151
user/src/com/google/gwt/junit/JUnitShell.java:151: OptionProxyTo {
eh, it came from auto-format ...
I'll change it manually.
On 2010/11/09 02:51:11, jat wrote:
Formatting
http://gwt-code-reviews.appspot.com/1043801/diff/1/8#newcode1140
user/src/com/google/gwt/junit/JUnitShell.java:1140:
mapping.setServletName(defaultServlet.getName());
I don't override the existing default servlet, be it the one provided by
Jetty, or provided by user code.
Instead, I add a filter only for the existing default servlet and it is
tried after the default servlet fails. More explanations in ProxyFilter
JavaDoc.
On 2010/11/09 02:51:11, jat wrote:
Where do you override the existing default servlet?
http://gwt-code-reviews.appspot.com/1043801/diff/1/8#newcode1536
user/src/com/google/gwt/junit/JUnitShell.java:1536: // TODO: figure out
if it is possible to allow multiple modules.
I'm not sure about it -- E2E tests run for the whole application, so (to
my limited experience) there's only one module which contains entry
point.
OTOH, I'm experimenting with initializing Module Under Test in test
methods, so this will be addressed in next pass.
On 2010/11/09 02:51:11, jat wrote:
I think this needs to support multiple modules.
http://gwt-code-reviews.appspot.com/1043801/diff/1/13
File user/src/com/google/gwt/junit/client/GWTTestCase.java (right):
http://gwt-code-reviews.appspot.com/1043801/diff/1/13#newcode199
user/src/com/google/gwt/junit/client/GWTTestCase.java:199: //
implemented in the translatable version of this class
user/super/com/google/gwt/junit/translatable/com/google/gwt/junit/client/GWTTestCase.java
This change is for moving GWTRunner from super to src (for easy
debugging). The format conforms to existing practice in the same file.
On 2010/11/09 02:51:11, jat wrote:
Where is the translatable version?
http://gwt-code-reviews.appspot.com/1043801/diff/1/14
File user/src/com/google/gwt/junit/client/TestService.java (right):
http://gwt-code-reviews.appspot.com/1043801/diff/1/14#newcode25
user/src/com/google/gwt/junit/client/TestService.java:25: public
interface TestService {
Naming is always hard. Any suggestions?
On 2010/11/09 02:51:11, jat wrote:
The name could probably be improved.
http://gwt-code-reviews.appspot.com/1043801/diff/1/14#newcode29
user/src/com/google/gwt/junit/client/TestService.java:29: void
click(HasClickHandlers hasClickHandlers);
It'll grow -- this is the main place for the future -- for example,
type, select, dragDrop, and wait ...
On 2010/11/09 02:51:11, jat wrote:
It seems odd that all we have here is click.
http://gwt-code-reviews.appspot.com/1043801/diff/1/16
File user/src/com/google/gwt/junit/client/impl/GWTRunner.java (right):
http://gwt-code-reviews.appspot.com/1043801/diff/1/16#newcode45
user/src/com/google/gwt/junit/client/impl/GWTRunner.java:45:
AsyncCallback<InitialResponse> {
Yes, exactly the same. The old version is a dummy stub. We needed the
translatable version just because the src-version GWTTestCase did not
have __doRunTest(), and this change adds it.
On 2010/11/09 02:51:11, jat wrote:
Is this mostly the same as the contents of the translatable version
that was
deleted?
Why do we no longer need a separate translatable version?
http://gwt-code-reviews.appspot.com/1043801/diff/1/18
File user/src/com/google/gwt/junit/server/ProxyFilter.java (right):
http://gwt-code-reviews.appspot.com/1043801/diff/1/18#newcode37
user/src/com/google/gwt/junit/server/ProxyFilter.java:37: * EXPERIMENTAL
and subject to change.
I'd encourage the brave and advanced users to use it (which has no
analogue within GWT SDK), but don't want to be blamed for lacking
advanced support -- I didn't try it with HTTPS, session, etc.; only
plain HTTP.
On 2010/11/09 02:51:11, jat wrote:
What is experimental about it? Since external users have been
requesting the
ability to proxy to an external server, is there any reason we would
encourage
them to use this?
http://gwt-code-reviews.appspot.com/1043801/diff/1/18#newcode127
user/src/com/google/gwt/junit/server/ProxyFilter.java:127: return
replaceModuleName ? uri.replaceFirst(syntheticModuleName,
On 2010/11/09 02:51:11, jat wrote:
replaceFirst takes regexes, but you are using literals. I think this
would be
better implemented like:
String uri = super.getRequestURI();
if (uri.startsWith("/") && syntheticModuleName != null) {
int n = syntheticModuleName.length();
if (syntheticModuleName.equals(uri.substring(1, n + 1))) {
uri = "/" + moduleName + uri.substring(n + 1);
}
}
return uri;
Done.
http://gwt-code-reviews.appspot.com/1043801/diff/1/18#newcode158
user/src/com/google/gwt/junit/server/ProxyFilter.java:158:
context.log("ProxyFilter.doFilter on proxied URL");
The new URL will be logged by Jetty later in its ProxyServlet.
On 2010/11/09 02:51:11, jat wrote:
Better log message.
http://gwt-code-reviews.appspot.com/1043801/diff/1/18#newcode168
user/src/com/google/gwt/junit/server/ProxyFilter.java:168: public void
setModuleName(String moduleName, String syntheticModuleName) {
On 2010/11/09 02:51:11, jat wrote:
Javadoc
Done.
http://gwt-code-reviews.appspot.com/1043801/diff/1/18#newcode171
user/src/com/google/gwt/junit/server/ProxyFilter.java:171:
context.log("ProxyFilter.setModuleName(" + moduleName + ", "
removed.
On 2010/11/09 02:51:11, jat wrote:
Does this need to be logged? I am not sure what level this gets
logged at, but
if it is INFO then I don't think we want to spam the log with messages
like
this.
If so, it needs a better message.
http://gwt-code-reviews.appspot.com/1043801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
