[gwt-contrib] Fix testShardsAreSomewhatBalanced to only check for shards that are too big. (issue1578810)
Reviewers: rdayal,
Description:
Fix testShardsAreSomewhatBalanced to only check for shards that are too
big.
(Small or empty shards are okay, and apparently they happen on Windows
and
Mac. Not sure if it's due to String.hashCode() being different or
garbage
collection.)
Please review this at http://gwt-code-reviews.appspot.com/1578810/
Affected files:
M dev/core/test/com/google/gwt/dev/util/StringInternerTest.java
Index: dev/core/test/com/google/gwt/dev/util/StringInternerTest.java
===
--- dev/core/test/com/google/gwt/dev/util/StringInternerTest.java (revision
10725)
+++ dev/core/test/com/google/gwt/dev/util/StringInternerTest.java (working
copy)
@@ -58,22 +58,30 @@
int[] shardSizes = interner.getShardSizes();
assertEquals(StringInterner.SHARD_COUNT, shardSizes.length);
-int minShardSize = Integer.MAX_VALUE;
+int meanShardSize = sum(shardSizes) / shardSizes.length;
+int expectedMaxShardSize = meanShardSize * 2;
+
int maxShardSize = 0;
-int total = 0;
+int tooBigShardCount = 0;
for (int candidate : shardSizes) {
- minShardSize = Math.min(minShardSize, candidate);
maxShardSize = Math.max(maxShardSize, candidate);
- total += candidate;
+ if (candidate > expectedMaxShardSize) {
+tooBigShardCount++;
+ }
}
-int meanShardSize = total / shardSizes.length;
-int expectedMinShardSize = meanShardSize / 5;
-int expectedMaxShardSize = meanShardSize * 5;
-if (minShardSize < expectedMinShardSize || maxShardSize >
expectedMaxShardSize) {
- fail("unbalanced shards: [" + minShardSize + "," + maxShardSize + "]
not within [" +
- expectedMinShardSize + ", " + expectedMaxShardSize + "]");
+if (tooBigShardCount > 0) {
+ fail(tooBigShardCount + " of " + shardSizes.length + " shards are
too big (more than " +
+ expectedMaxShardSize + " entries); largest shard has " +
maxShardSize + " entries.");
}
+ }
+
+ private static int sum(int[] ints) {
+int total = 0;
+for (int i : ints) {
+ total += i;
+}
+return total;
}
private static String repeat(char c, int length) {
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing a bug in HasDataPresenter where changes to the temporary state aren't propagated to the n... (issue1585803)
committed as r10731 http://gwt-code-reviews.appspot.com/1585803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing a bug in HasDataPresenter where changes to the temporary state aren't propagated to the n... (issue1585803)
http://gwt-code-reviews.appspot.com/1585803/diff/4/user/src/com/google/gwt/user/cellview/client/CellBrowser.java
File user/src/com/google/gwt/user/cellview/client/CellBrowser.java
(right):
http://gwt-code-reviews.appspot.com/1585803/diff/4/user/src/com/google/gwt/user/cellview/client/CellBrowser.java#newcode217
user/src/com/google/gwt/user/cellview/client/CellBrowser.java:217: * The
currently selected value in this list.
On 2011/10/31 17:31:39, skybrian wrote:
s/list/tree/
List is correct in this case. This is the inner BrowserCellList class.
We keep track of the selected value in each list so we can deselect it
as needed.
http://gwt-code-reviews.appspot.com/1585803/diff/4/user/src/com/google/gwt/user/cellview/client/CellBrowser.java#newcode1081
user/src/com/google/gwt/user/cellview/client/CellBrowser.java:1081: //
Always use the ENABLED keyboard selection policy within each list. The
On 2011/10/31 17:31:39, skybrian wrote:
Didn't get it the first time. How about:
"A CellBrowser has a single selection policy and multiple lists, so
we're not
using the selection policy in each list. Just leave them on all the
time."
Since we're not using it, does it matter whether it's on or off? Also,
can user
code try to change the list's selection policy? It seems like that
would either
have no effect or cause something to break. Maybe it should throw an
exception
to avoid confusion?
We use it to keep track of the selected/open item at each level in the
browser. Users cannot access the individual CellLists, so they can't
change it.
http://gwt-code-reviews.appspot.com/1585803/diff/4/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
File user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
(right):
http://gwt-code-reviews.appspot.com/1585803/diff/4/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java#newcode1062
user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java:1062:
private boolean resolvePendingState(JsArrayInteger modifiedRows) {
On 2011/10/31 17:31:39, skybrian wrote:
It's unclear to me where the potentially recursive method calls are.
It's
probably a good idea to add comments at those method calls warning
about this.
Done.
Also, it's easy to confuse "pending" versus "pendingState". Maybe
there should
be more distinct names for these?
Switching pending to newState, because it will become the state within
this resolvePendingState method.
http://gwt-code-reviews.appspot.com/1585803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing a bug in HasDataPresenter where changes to the temporary state aren't propagated to the n... (issue1585803)
LGTM
I like the test and understand the CellBrowser change. The
resolvePendingState() method is hard to verify and I wonder if there are
any other lurking bugs, but I'm taking your word for it.
http://gwt-code-reviews.appspot.com/1585803/diff/4/user/src/com/google/gwt/user/cellview/client/CellBrowser.java
File user/src/com/google/gwt/user/cellview/client/CellBrowser.java
(right):
http://gwt-code-reviews.appspot.com/1585803/diff/4/user/src/com/google/gwt/user/cellview/client/CellBrowser.java#newcode217
user/src/com/google/gwt/user/cellview/client/CellBrowser.java:217: * The
currently selected value in this list.
s/list/tree/
http://gwt-code-reviews.appspot.com/1585803/diff/4/user/src/com/google/gwt/user/cellview/client/CellBrowser.java#newcode1081
user/src/com/google/gwt/user/cellview/client/CellBrowser.java:1081: //
Always use the ENABLED keyboard selection policy within each list. The
Didn't get it the first time. How about:
"A CellBrowser has a single selection policy and multiple lists, so
we're not using the selection policy in each list. Just leave them on
all the time."
Since we're not using it, does it matter whether it's on or off? Also,
can user code try to change the list's selection policy? It seems like
that would either have no effect or cause something to break. Maybe it
should throw an exception to avoid confusion?
http://gwt-code-reviews.appspot.com/1585803/diff/4/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
File user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
(right):
http://gwt-code-reviews.appspot.com/1585803/diff/4/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java#newcode1062
user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java:1062:
private boolean resolvePendingState(JsArrayInteger modifiedRows) {
It's unclear to me where the potentially recursive method calls are.
It's probably a good idea to add comments at those method calls warning
about this.
Also, it's easy to confuse "pending" versus "pendingState". Maybe there
should be more distinct names for these?
http://gwt-code-reviews.appspot.com/1585803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing a bug in HasDataPresenter where changes to the temporary state aren't propagated to the n... (issue1585803)
I updated the patch to actually fix the problem and added a test case that fails without the fix but passes with it. I was getting frustrated on Friday and wanted to get something out, but it didn't fix the underlying problem correctly. After thinking about the CellBrowser bug for the weekend, I realized that CellBrowser has to handle selection itself. We can't control selection from within each individual List because they compete with each other. http://gwt-code-reviews.appspot.com/1585803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing a bug in HasDataPresenter where changes to the temporary state aren't propagated to the n... (issue1585803)
http://gwt-code-reviews.appspot.com/1585803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Announce: GwtQuery 1.1.0
As jquery, gquery only works with single numeric values. Additionally Gquery core supports color and html-attributes (useful for svg) animations. I think, there is a jquery plugin to animate the background-position, although is not ported to gquery. You could use background-position-y and background-position-x css properties, but it could have issues with some browsers. Maybe you are interested on sending a patch or a plugin for that with your function :-) Regards - Manolo On Fri, Oct 28, 2011 at 4:07 PM, Christian Goudreau wrote: > Btw I made a mistake, It only happened on background-position :D > Cheers, > > On Fri, Oct 28, 2011 at 10:05 AM, Christian Goudreau > wrote: >> >> First, GREAT WORK! I totally find this library AWESOME! >> Quick question, does animating background-position for example work now in >> the new version? Last time I checked, it was impossible to animate two css >> properties and had to make my own function :D >> Cheers, >> >> On Thu, Oct 27, 2011 at 11:16 PM, Ray Cromwell >> wrote: >>> >>> Awesome job Manuel, taking this project from a proof of concept to a >>> rock solid library! >>> >>> -Ray >>> >>> >>> On Wed, Oct 26, 2011 at 10:38 AM, Manuel Carrasco Moñino >>> wrote: >>> > The GQuery team is proud to announce the version 1.1.0 of the library. >>> > >>> > We have been working hard in order to fix many issues and to achieve a >>> > closed jQuery API. >>> > >>> > The new release includes new features like the jQuery Ajax API, Data >>> > binding to easily manage json and xml data, better interaction with >>> > gwt widgets, and improved documentation. >>> > >>> > You can see the full list of features in the presentation given in The >>> > RivieraDev last week in France [1] or taking a look to the project >>> > wiki pages [2] >>> > >>> > [1] >>> > http://www.slideshare.net/dodotis/gquery-a-jquery-clone-for-gwt-rivieradev-2011 >>> > [2] http://code.google.com/p/gwtquery/w/list >>> > >>> > >>> > Thanks so much to the people sending issues, testing, and using the >>> > library. >>> > >>> > - GQuery Team >>> > >>> > -- >>> > http://groups.google.com/group/Google-Web-Toolkit-Contributors >>> > >>> >>> -- >>> http://groups.google.com/group/Google-Web-Toolkit-Contributors >> >> >> -- >> Christian Goudreau >> www.arcbees.com > > > > -- > Christian Goudreau > www.arcbees.com > > -- > http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: Re: [gwt-contrib] Aw: Regression: "instanceof" compiler issue
I've retested with gwt 2.4 and the issue seems to be fixed. Thanks! -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
