-
Notifications
You must be signed in to change notification settings - Fork 387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLDR-17287 basic performance test for the back end #3418
Conversation
- execute a bunch of votes all in a row, which calls STFactory.PerLocaleData.voteForvalueWithType()
@btangmu this is a start, i'm redoing it to go through VoteAPIHelper! |
tools/cldr-apps/src/test/java/org/unicode/cldr/unittest/web/TestSTFactory.java
Outdated
Show resolved
Hide resolved
tools/cldr-apps/src/test/resources/org/unicode/cldr/unittest/web/data/TestVotePerf.xml
Show resolved
Hide resolved
- refactor VoteAPIHelper to be callable from unit tests outside of a web server - improve error results from ST
@btangmu i'm getting the 'read' side going too! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok if TODOs get done in subsequent pr
tools/cldr-apps/src/test/java/org/unicode/cldr/unittest/web/TestSTFactory.java
Show resolved
Hide resolved
tools/cldr-apps/src/test/java/org/unicode/cldr/unittest/web/TestSTFactory.java
Outdated
Show resolved
Hide resolved
tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/VoteAPIHelper.java
Outdated
Show resolved
Hide resolved
- update apiverify to verify via API
4a10a97
to
7bc7af9
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
- refactor to subroutines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving reluctantly, see comment about deep nesting due to unnamed handler that needs to be named instead of being stuffed inside the parentheses in myReader.setHandler(...)
break; | ||
default: | ||
throw new IllegalArgumentException( | ||
"Unknown test element type " + elem); | ||
} | ||
} | ||
|
||
private void handleElementVerify( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code is too deeply nested! the root of the problem starts with myReader.setHandler(...)
followed by all the code for the unnamed handler being within the parentheses. it would be sooooo much better to give the handler a name, and just put its name in the parentheses, and define the handler as a named class outside of runDataDrivenTest
to prevent runDataDrivenTest
from being 600+ lines long
the deep nesting probably causes the formatter to insert more newlines, adding insult to injury
CLDR-17287
ALLOW_MANY_COMMITS=true