Skip to content

test: refactor UnicodeJspsTest and create unicodetools-testutils #209

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Mar 12, 2022
Merged

test: refactor UnicodeJspsTest and create unicodetools-testutils #209

merged 5 commits into from
Mar 12, 2022

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Mar 10, 2022

  • create module unicodetools-testutils for shared test utils
  • move UnicodeJspsTest into UnicodeJsps/src/test/java

For: #146

@srl295
test: refactor UnicodeJspsTest and create unicodetools-testutils
4c98cae 
- create module unicodetools-testutils for shared test utils
- move UnicodeJspsTest into UnicodeJsps/src/test/java

For: #146

test: more UnicodeJspsTest updates

For: #146
@srl295 srl295 requested a review from macchiati March 10, 2022 23:13
@srl295
Copy link
Member Author

srl295 commented Mar 10, 2022

@macchiati and anyone: test failures start here: https://github.com/unicode-org/unicodetools/runs/5503791433?check_suite_focus=true#step:10:904

@srl295
test: more UnicodeJspsTest updates
0b4dc47 
Make some tests skipped unless -DUNICODETOOLS_RUN_BROKEN_TEST=true

For: #146
@srl295 srl295 self-assigned this Mar 10, 2022
@srl295
Copy link
Member Author

srl295 commented Mar 10, 2022

tests should pass… more fixes can be done in the future.

srl295
srl295 commented Mar 10, 2022
View reviewed changes
UnicodeJsps/src/test/java/org/unicode/jsptest/TestLanguageid.java
results = LanguageCode.validate("$, eng-840, fr-fr",new ULocale("en"));
assertTrue("", results.contains("target='languageid'>fr-FR</b>"));
assertContains(results, "target='languageid'>fr-FR</a></b>");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@macchiati this seemed like a simple data fix in the assertion, maybe the HTML just changed?

markusicu
markusicu reviewed Mar 11, 2022
View reviewed changes
Copy link
Member

@markusicu markusicu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes look plausible so far... thanks for doing all of this work!

@srl295 @markusicu
Update unicodetools-testutils/src/main/java/org/unicode/unittest/Test…
15c21b2 
…FmwkMinusMinus.java

Co-authored-by: Markus Scherer <markus.icu@gmail.com>
@srl295
Copy link
Member Author

srl295 commented Mar 11, 2022

changes look plausible so far... thanks for doing all of this work!

Thanks. Fixed indentation, ok to merge ?

@srl295
Copy link
Member Author

srl295 commented Mar 11, 2022 via email

@srl295 srl295 requested a review from markusicu March 11, 2022 23:43
markusicu
markusicu approved these changes Mar 12, 2022
View reviewed changes
Copy link
Member

@markusicu markusicu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm tnx

@srl295 srl295 merged commit d430a9a into main Mar 12, 2022
@srl295 srl295 deleted the srl295/issue146 branch March 12, 2022 02:01
@markusicu markusicu mentioned this pull request May 11, 2022
Closed
2 tasks
@srl295 srl295 mentioned this pull request May 31, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markusicu markusicu markusicu approved these changes

@macchiati macchiati Awaiting requested review from macchiati

@echeran echeran Awaiting requested review from echeran

@josh-hadley josh-hadley Awaiting requested review from josh-hadley

Assignees

@srl295 srl295

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@srl295 @markusicu