Skip to content
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

Make date-formatting tests less fragile with regular expressions. #2450

Merged
merged 4 commits into from
Jul 26, 2023
Merged

Make date-formatting tests less fragile with regular expressions. #2450

merged 4 commits into from
Jul 26, 2023

Conversation

eamonnmcmanus
Copy link
Member

@eamonnmcmanus eamonnmcmanus commented Jul 26, 2023 •
edited

This is not great. We should really ensure that formatted dates are the same regardless of JDK version. There is code that attempts to do that but it is not really effective. So for we use regular expressions to paper over the differences.

Purpose

Description

Checklist

  • New code follows the Google Java Style Guide
  • If necessary, new public API validates arguments, for example rejects null
  • New public API has Javadoc
    • Javadoc uses @since $next-version$
      ($next-version$ is a special placeholder which is automatically replaced during release)
  • If necessary, new unit tests have been added
    • Assertions in unit tests use Truth, see existing tests
    • No JUnit 3 features are used (such as extending class TestCase)
    • If this pull request fixes a bug, a new test was added for a situation which failed previously and is now fixed
  • mvn clean verify javadoc:jar passes without errors

@eamonnmcmanus
Make date-formatting tests less fragile with regular expressions.
e06bb0c 
This is not great. We should really ensure that formatted dates are the same
regardless of JDK version. There is code that attempts to do that but it is not
really effective. So for now we fudge around the differences by using regular
expressions to paper over the differences.
@eamonnmcmanus eamonnmcmanus mentioned this pull request Jul 26, 2023
Merged
Marcono1234
Marcono1234 approved these changes Jul 26, 2023
View reviewed changes
Copy link
Collaborator

@Marcono1234 Marcono1234 left a comment •
edited

Choose a reason for hiding this comment

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

Thanks, looks good!

For future reference, for which JDK version did you encounter the "Narrow No Break Space" in front of AM / PM?

@eamonnmcmanus
Copy link
Member Author

For future reference, for which JDK version did you encounter the "Narrow No Break Space" in front of AM / PM?

It was Google's JDK 11, which might have some localization differences from regular JDK 11. (I don't know, but that's my theory.)

@eamonnmcmanus eamonnmcmanus merged commit 5055b62 into google:main Jul 26, 2023
8 checks passed
@eamonnmcmanus eamonnmcmanus deleted the tests branch July 26, 2023 19:24
eamonnmcmanus added a commit that referenced this pull request Jul 30, 2023
@Marcono1234 @dependabot
@elevne @eamonnmcmanus
strictness2 fixes & improvements (#2456)
077cbce 
* Adjust ProGuard default rules and shrinking tests (#2420)

* Adjust ProGuard default rules and shrinking tests

* Adjust comment

* Add shrinking test for class without no-args constructor; improve docs

* Improve Unsafe mention in Troubleshooting Guide

* Improve comment for `-if class *`

* Bump com.google.guava:guava from 32.0.1-jre to 32.1.1-jre (#2444)

Bumps [com.google.guava:guava](https://github.com/google/guava) from 32.0.1-jre to 32.1.1-jre.
- [Release notes](https://github.com/google/guava/releases)
- [Commits](https://github.com/google/guava/commits)

---
updated-dependencies:
- dependency-name: com.google.guava:guava
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump com.google.guava:guava-testlib from 32.0.1-jre to 32.1.1-jre (#2443)

Bumps [com.google.guava:guava-testlib](https://github.com/google/guava) from 32.0.1-jre to 32.1.1-jre.
- [Release notes](https://github.com/google/guava/releases)
- [Commits](https://github.com/google/guava/commits)

---
updated-dependencies:
- dependency-name: com.google.guava:guava-testlib
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Support non-generic type for `TypeToken.getParameterized` for legacy reasons (#2447)

This partially restores the behavior before a589ef2,
except that back then for a non-generic type a bogus `TypeToken(ParameterizedType)`
was created, whereas now a `TypeToken(Class)` is created instead.

* Fixed Typo in GsonBuilder.java (#2449)

* Make date-formatting tests less fragile with regular expressions. (#2450)

* Make date-formatting tests less fragile with regular expressions.

This is not great. We should really ensure that formatted dates are the same
regardless of JDK version. There is code that attempts to do that but it is not
really effective. So for now we fudge around the differences by using regular
expressions to paper over the differences.

* Temporarily add test-debugging code.

* Another attempt at debugging a test failure.

* Fix pattern in assertion.

* Modification in test cases (#2454)

* Fixed Typo in GsonBuilder.java

* Suggestions on Test cases

* Modified test cases using assertThrows method (JUnit)

* Update gson/src/test/java/com/google/gson/JsonArrayAsListTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/GsonTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/JsonArrayAsListTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/JsonStreamParserTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/JsonStreamParserTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/JsonStreamParserTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/ToNumberPolicyTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/TypeAdapterTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/TypeAdapterTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/ToNumberPolicyTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/ToNumberPolicyTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

---------

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Minor follow-up changes

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: elevne <97422844+elevne@users.noreply.github.com>
Co-authored-by: Éamonn McManus <emcmanus@google.com>
Co-authored-by: Wonil <cwi5525@naver.com>
eamonnmcmanus added a commit that referenced this pull request Jul 30, 2023
@eamonnmcmanus @marten-voorberg
@gustajoh @LMC117 @Marcono1234 @dependabot @elevne
Strict mode for JSON parsing, contributed by @marten-voorberg. (#2437)
b3dce2d 
* Strict mode for JSON parsing (#2323)

* Feat #6: Add strict flag to Gson and GsonBuilder

* Test #2: Add failing tests for capitalized keywords

* Feat #2: JsonReader does not read (partially) capitalized keywords if strict mode is used

* Feat #3: Added implementation and tests for JSONReader not accepting specific escape sequence representing in strict mode

* Test #3: Simplify test cases by removing unnecessary array

* Feat #3: Improve error by including the illegal character

* Feat #5: JsonReader does not allow unespaced control flow characters in strict mode

* Test #5: Test unespaced control flow characters in strict mode

* Feat #4: Disallow espaced newline character in strict mode

* Test #4: Add tests for (dis)allowing newline character depensding on strictness

* Test #5: Test case for unescaped control char in non-strict mode

* Test #2: Simplify test cases

* Feat #13: Change leniency API to Strictness enum in JsonReader, Gson, and GsonBuilder

* Feat #15: Change JsonWriter API to also use Strictness

* Test #15: Test Strictness in JsonWriter API

* Doc #15: Add and update documentation for Strictness in JsonWriter API

* refactor #12: Fixed typos and empty catch brackets in tests

* refactor #12: Resolved importing wildcards, made some lines adhere to Google java style

* #5 Add test case for unescaped control characters

* Feat #5: add new lines to make JsonReader able to detect unescaped control characters (U+0000 through U+001F) and throw exceptions.

* Feat #5: add new lines to make JsonReader able to detect unescaped control characters (U+0000 through U+001F) and throw exceptions.

* Test #11: Added two tests for testing implementation of control character handling in strict mode and moved the implementation to nextQuotedValue

* Test #11: Added two tests for testing implementation of control character handling in strict mode and moved the implementation to nextQuotedValue

---------

Co-authored-by: LMC117 <2295699210@qq.com>
Co-authored-by: Marten Voorberg <martenvoorberg@gmail.com>

* Doc #17: Add and change javadoc of public methods

* Doc #17: Update JavaDoc in JsonReader and Strictness

* Doc #17: Update JavaDoc in Gson and GsonBuilder

* Test #34: Add tests for setting strictness through GsonBuilder

* Fix: Add Fix broken test

* Fix: Invalid JavaDoc in Gson.java

* Doc #17: update outdated javadoc

* #37: Resolve more PR feedback 

* Fix #37: Resolve various PR comments

* Fix #37: Resolve various PR comments

* Refactor #35: Refactor JsonReader#peekKeyword to reduce the amount of strictness checks (#39)

* Doc #40: Update JavaDoc based on PR feedback

* Doc #40: Update old RFC in GsonBuilder documentation

* Doc #40: Fix formatting error in JavaDoc

* Doc #40: Add tests for setting strictness and lenient to JsonReaderTest

* Test #43: Changed tests to make use of assertThrows

* test #43: Changed tests to make use of assertThrows as per feedback

* Test #43: Update JsonWriterTest#testStrictnessNull to use assertThrows

* Test #43: Update JsonWriterTest#testStrictnessNull to use assertThrows

* test #43: Resolve PR recommendations

* Test #43: Mini change to TC

* Test #43: Mini change to TC

---------

Co-authored-by: Marten Voorberg <martenvoorberg@gmail.com>

* doc #46: Resolved comments in main PR

* Feat #45: Change Gson.fromJson and Gson.toJson to be strict when the provided writer/reader is strict

* Fix #45: Small type

* Update gson/src/test/java/com/google/gson/stream/JsonReaderTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Fix #45: Resolve various comments by Marcono1234

* Update gson/src/main/java/com/google/gson/GsonBuilder.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Fix #45: Resolve various comments by Marcono1234

* Fix #45: Resolve various comments by eamonmcmanus

* Strictness mode follow-up

* Update Troubleshooting.md and Gson default lenient mode documentation

* Always use GSON strictness when set.

* Rename Strictness.DEFAULT to Strictness.LEGACY_STRICT

* Update JavaDoc with new strictness functionality

* Replace default with legacy strict for JsonReader javadoc

* Add JSONReader test cases for U2028 and U2029

* Refactor JSONReader#peekKeyWord() based on @eamonmcmanus's suggestion

* Deprecate setLenient in favor of setStrictness

---------

Co-authored-by: Carl Peterson <unknown>
Co-authored-by: Gustaf Johansson <gustajoh@kth.se>
Co-authored-by: gustajoh <58432871+gustajoh@users.noreply.github.com>
Co-authored-by: LMC117 <2295699210@qq.com>
Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Strictness follow-up (#2408)

* Strictness mode follow-up

- Remove mentions of `null` Gson strictness; this is an implementation detail
- Fix incorrect / outdated documentation
- Reduce links to RFC; if there is already a link to it in a previous sentence
  don't link to it again
- Extend and update tests
- Minor punctuation changes in documentation for consistency

* Deprecate `setLenient` methods

* `strictness2` fixes & improvements (#2456)

* Adjust ProGuard default rules and shrinking tests (#2420)

* Adjust ProGuard default rules and shrinking tests

* Adjust comment

* Add shrinking test for class without no-args constructor; improve docs

* Improve Unsafe mention in Troubleshooting Guide

* Improve comment for `-if class *`

* Bump com.google.guava:guava from 32.0.1-jre to 32.1.1-jre (#2444)

Bumps [com.google.guava:guava](https://github.com/google/guava) from 32.0.1-jre to 32.1.1-jre.
- [Release notes](https://github.com/google/guava/releases)
- [Commits](https://github.com/google/guava/commits)

---
updated-dependencies:
- dependency-name: com.google.guava:guava
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump com.google.guava:guava-testlib from 32.0.1-jre to 32.1.1-jre (#2443)

Bumps [com.google.guava:guava-testlib](https://github.com/google/guava) from 32.0.1-jre to 32.1.1-jre.
- [Release notes](https://github.com/google/guava/releases)
- [Commits](https://github.com/google/guava/commits)

---
updated-dependencies:
- dependency-name: com.google.guava:guava-testlib
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Support non-generic type for `TypeToken.getParameterized` for legacy reasons (#2447)

This partially restores the behavior before a589ef2,
except that back then for a non-generic type a bogus `TypeToken(ParameterizedType)`
was created, whereas now a `TypeToken(Class)` is created instead.

* Fixed Typo in GsonBuilder.java (#2449)

* Make date-formatting tests less fragile with regular expressions. (#2450)

* Make date-formatting tests less fragile with regular expressions.

This is not great. We should really ensure that formatted dates are the same
regardless of JDK version. There is code that attempts to do that but it is not
really effective. So for now we fudge around the differences by using regular
expressions to paper over the differences.

* Temporarily add test-debugging code.

* Another attempt at debugging a test failure.

* Fix pattern in assertion.

* Modification in test cases (#2454)

* Fixed Typo in GsonBuilder.java

* Suggestions on Test cases

* Modified test cases using assertThrows method (JUnit)

* Update gson/src/test/java/com/google/gson/JsonArrayAsListTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/GsonTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/JsonArrayAsListTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/JsonStreamParserTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/JsonStreamParserTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/JsonStreamParserTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/ToNumberPolicyTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/TypeAdapterTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/TypeAdapterTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/ToNumberPolicyTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Update gson/src/test/java/com/google/gson/ToNumberPolicyTest.java

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

---------

Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>

* Minor follow-up changes

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: elevne <97422844+elevne@users.noreply.github.com>
Co-authored-by: Éamonn McManus <emcmanus@google.com>
Co-authored-by: Wonil <cwi5525@naver.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Marten <martenvoorberg@gmail.com>
Co-authored-by: Gustaf Johansson <gustajoh@kth.se>
Co-authored-by: gustajoh <58432871+gustajoh@users.noreply.github.com>
Co-authored-by: LMC117 <2295699210@qq.com>
Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: elevne <97422844+elevne@users.noreply.github.com>
Co-authored-by: Wonil <cwi5525@naver.com>
@Marcono1234 Marcono1234 mentioned this pull request Aug 17, 2023
Open
@Marcono1234 Marcono1234 mentioned this pull request Dec 27, 2023
Closed
4 tasks
@Marcono1234 Marcono1234 mentioned this pull request May 29, 2024
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Marcono1234 Marcono1234 Marcono1234 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@eamonnmcmanus @Marcono1234