[SR-10574] Add tests for new Japanese calendar era name #2200

novi · 2019-04-30T14:43:16Z

Add Calendar test
Add DateFormatter test

Test Calendar and DateFormatter with new era name (SR-10574).
Related swiftlang/swift#24387.

ianpartridge · 2019-04-30T14:47:00Z

@swift-ci test

novi · 2019-04-30T15:05:26Z

Reiwa(令和) era has just begun 😄.

TestFoundation/TestCalendar.swift

spevans · 2019-04-30T15:14:24Z

TestFoundation/TestDateFormatter.swift

-        // format test
-        let dateString = formatter.string(from: Date(timeIntervalSince1970: 1556633400)) // April 30, 2019, 11:10 PM (JST)
-        XCTAssertEqual(dateString, "平成31年4月30日 23:10")
+// Disabled until the ICU is updated.


Could you enable these tests please. I just want to check they fail with the current ICU so then they pass with the new ICU we know its all working correctly, thanks.

@spevans This following codes require ICU 64.2. It should be enabled?
And if the ICU is 64.2, L448-L457 will fail. This lines for ICU 61.2.

We cannot commit tests that fail.

If you want, you can replace the do block with if shouldAttemptXFailTests("These tests require ICU 64.2 or later. https://bugs.swift.org/browse/SR-XXXX") { … } where the XXXX link is a bug filed on bugs.swift.org to migrate to newer ICU. @spevans, you can then run this test by setting the environment variable NS_FOUNDATION_ATTEMPT_XFAIL_TESTS to YES.

millenomi · 2019-04-30T17:23:02Z

TestFoundation/TestCalendar.swift

+        do {
+            let date = Date(timeIntervalSince1970: 1556633400) // April 30, 2019
+            let components = calendar.dateComponents([.era, .year, .month, .day], from: date)
+            XCTAssertEqual(calendar.eraSymbols[components.era!], "Heisei")


Is this assuming a particular current locale?

spevans · 2019-04-30T18:04:38Z

This is to be tested with swiftlang/swift#24387

novi · 2019-05-01T06:41:39Z

I fixed some lines of the test.

Avoid using force unwrapping
Add a Locale to the Calendar test
Replace the block for ICU 64.2

Is it okay? Would we these commits combine into a single commit?

spevans · 2019-05-01T06:45:12Z

Yes, if you could, can rebase squash them into one commit, thanks.

novi · 2019-05-01T07:07:31Z

It seems to be okay.

spevans · 2019-05-01T07:26:48Z

@swift-ci test

millenomi · 2019-05-01T08:10:28Z

@swift-ci please test

spevans · 2019-05-01T12:55:32Z

Please test with the following pull request:
swiftlang/swift#24387

@swift-ci test linux

spevans · 2019-05-02T07:15:44Z

@ianpartridge It looks like the CI isnt pulling in the new version of ICU when doing update-checkout but I dont know what the fix for that is.

ianpartridge · 2019-05-02T09:34:31Z

Yeah I'm not sure there is one. It looks like it decides which release of ICU to use before it looks at my PR :( Maybe we'll have to merge then test?

millenomi · 2019-05-03T17:09:58Z

@shahmishal Can you look into why CI isn't pulling the new updated ICU from the associated Swift PR?

shahmishal · 2019-05-03T17:26:38Z

Please test with the following pull request:
swiftlang/swift#24387

@swift-ci test linux

novi · 2019-05-08T07:20:45Z

The test crashes on something related CF and Foundation bridges.
Does anyone know why this error is caused?

novi · 2019-05-08T07:30:04Z

I found that Calendar.eraSymbols for any calendars cause crash with following code on Swift version 5.0.1 (swift-5.0.1-RELEASE) (Docker image tagged swift:bionic ) .

import Foundation
let cal = Calendar(identifier: .gregorian)
cal.eraSymbols

spevans · 2019-05-08T11:07:52Z

@novi, could you open a bug for that issue please. The actual error for it is:

Could not cast value of type 'Foundation._NSCFArray' (0x7fcaf94d7660) to '__C.CFArrayRef' (0x7fcaf94b3b80).

Caused by this line in NSCalendar.swift:

let result = (CFDateFormatterCopyProperty(dateFormatter, key) as! CFArray)._swiftObject

Thanks

novi · 2019-05-08T13:18:57Z

@spevans I submitted an issue. SR-10638

This pull request would be continued after the bug is fixed.

YOCKOW · 2019-06-08T05:11:48Z

@novi SR-10638 seems to be resolved. Could you go ahead if there is something to do?

novi · 2019-06-08T08:03:04Z

@YOCKOW Thanks. That's great.

@spevans @millenomi Could we test this pull request again?

ianpartridge · 2019-06-08T08:50:57Z

@swift-ci test

novi · 2019-06-09T09:41:15Z

@ianpartridge Thanks. Would it be tested with swiftlang/swift#24387 ?

ikesyo · 2019-06-09T10:18:39Z

swiftlang/swift#24387

@swift-ci Please test

novi · 2019-06-19T06:43:46Z

It seems to be not tested with swiftlang/swift#24387. Would it be tested with that?

ikesyo · 2019-06-19T09:48:06Z

@swift-ci Please test with the following pull request

swiftlang/swift#24387

novi · 2019-06-25T04:34:11Z

Thanks, but the latest testing is 15 days ago. Could we have any ideas?

spevans · 2019-11-16T21:47:48Z

@swift-ci test linux

spevans · 2019-11-17T13:58:07Z

@novi Now that ICU has been updated to 65.1 in swiftlang/swift#27937 if you could update this test to use XCTUnwrap instead of unwrapped() and remove the shouldAttemptXFailTests then this should be ok for merging, thanks.

novi · 2019-11-21T06:48:49Z

@spevans The tests is updated for ICU 65.1. (A branch name of my fork includes 61.2, but we could merge this PR anyway.)

spevans · 2019-11-21T08:15:11Z

@swift-ci test

novi · 2020-01-21T06:44:47Z

Could you merge this pull request? I would update to resolve this conflicts before merge.

spevans · 2020-01-21T10:10:40Z

Yes please rebase it and then it can be merged. ICU has been updated to 65.1 now so the tests should now pass.

novi · 2020-01-22T08:05:44Z

@spevans Thanks. I have rebased it and the base is up to date.

spevans · 2020-01-22T10:17:15Z

TestFoundation/TestCalendar.swift

@@ -111,6 +111,31 @@ class TestCalendar: XCTestCase {
        XCTAssertEqual(components.day, 18)

    }
+
+    func test_gettingDatesOnJapaneseCalendar() throws {


You'll need to add this test into the list of tests at the end of the file, Linux and TestFoundation doesnt automatically pick up tests as happens on macOS. If you can squash that change in we can get this merged, thanks

@spevans Sure. Fixed it. Now, It seems be okay.

spevans · 2020-01-22T18:10:14Z

@swift-ci test and merge

- Add Calendar test - Add DateFormatter test

novi · 2020-03-12T08:29:52Z

@spevans Hello, will it be tested? I have resolved the conflicts.

spevans · 2020-03-12T09:20:22Z

@swift-ci test linux

spevans reviewed Apr 30, 2019

View reviewed changes

TestFoundation/TestCalendar.swift Outdated Show resolved Hide resolved

spevans reviewed Apr 30, 2019

View reviewed changes

millenomi reviewed Apr 30, 2019

View reviewed changes

novi force-pushed the add-test-for-japanese-calendar-era-name-icu-61.2 branch from ae737c1 to 70d8d97 Compare May 1, 2019 07:05

YOCKOW mentioned this pull request Jun 6, 2019

SR-10638: Change the type of force casting in NSCalendar. #2318

Merged

novi force-pushed the add-test-for-japanese-calendar-era-name-icu-61.2 branch from 70d8d97 to e0cc72e Compare November 21, 2019 06:40

novi force-pushed the add-test-for-japanese-calendar-era-name-icu-61.2 branch from e0cc72e to dde1c44 Compare January 22, 2020 08:02

spevans reviewed Jan 22, 2020

View reviewed changes

novi force-pushed the add-test-for-japanese-calendar-era-name-icu-61.2 branch from dde1c44 to ddeeecb Compare January 22, 2020 18:01

Add tests for new Japanese calendar era name

0bfcda9

- Add Calendar test - Add DateFormatter test

novi force-pushed the add-test-for-japanese-calendar-era-name-icu-61.2 branch from ddeeecb to 0bfcda9 Compare March 12, 2020 08:28

spevans merged commit 69232d2 into swiftlang:master Mar 12, 2020

swift-ci mentioned this pull request Apr 30, 2019

[SR-10574] Update ICU on Linux to support new Japanese calendar era name swiftlang/swift#52974

Closed

swift-ci mentioned this pull request May 5, 2022

[SR-10638] Crashes on accessing property of Foundation.Calendar #3421

Closed

[SR-10574] Add tests for new Japanese calendar era name #2200

[SR-10574] Add tests for new Japanese calendar era name #2200

Uh oh!

Conversation

novi commented Apr 30, 2019

Uh oh!

ianpartridge commented Apr 30, 2019

Uh oh!

novi commented Apr 30, 2019

Uh oh!

Uh oh!

spevans Apr 30, 2019

Choose a reason for hiding this comment

Uh oh!

novi Apr 30, 2019

Choose a reason for hiding this comment

Uh oh!

millenomi Apr 30, 2019

Choose a reason for hiding this comment

Uh oh!

millenomi Apr 30, 2019

Choose a reason for hiding this comment

Uh oh!

spevans commented Apr 30, 2019

Uh oh!

novi commented May 1, 2019

Uh oh!

spevans commented May 1, 2019

Uh oh!

novi commented May 1, 2019

Uh oh!

spevans commented May 1, 2019

Uh oh!

millenomi commented May 1, 2019

Uh oh!

spevans commented May 1, 2019

Uh oh!

spevans commented May 2, 2019

Uh oh!

ianpartridge commented May 2, 2019

Uh oh!

millenomi commented May 3, 2019

Uh oh!

shahmishal commented May 3, 2019

Uh oh!

novi commented May 8, 2019

Uh oh!

novi commented May 8, 2019

Uh oh!

spevans commented May 8, 2019

Uh oh!

novi commented May 8, 2019

Uh oh!

YOCKOW commented Jun 8, 2019

Uh oh!

novi commented Jun 8, 2019

Uh oh!

ianpartridge commented Jun 8, 2019

Uh oh!

novi commented Jun 9, 2019

Uh oh!

ikesyo commented Jun 9, 2019

Uh oh!

novi commented Jun 19, 2019

Uh oh!

ikesyo commented Jun 19, 2019

Uh oh!

novi commented Jun 25, 2019

Uh oh!

spevans commented Nov 16, 2019

Uh oh!

spevans commented Nov 17, 2019

Uh oh!

novi commented Nov 21, 2019

Uh oh!

spevans commented Nov 21, 2019

Uh oh!

novi commented Jan 21, 2020

Uh oh!

spevans commented Jan 21, 2020