Strange behavior the same code fails when building a Cocoa CLI and succeeds in Swift-corelibs-foundation context.

Dear All,

I do encounter a very strange Issue.

I have been adding a test to `swift-corelibs-foundation/testJSONSerialization.swift`
This test succeed within swift-corelibs-foundation test, but fails when repackaged to a Cocoa CLI app.

The swift-corelibs-foundation test :

    func test_serializeFloatingPointWithANonDotDecimalSeparatorLocale() {
        // We use a locale that is using "," as decimal separator.
        setlocale(LC_ALL,"fr_FR")

        let dictionary = ["decimalValue":1.1]
        do{
            let data = try JSONSerialization.data(withJSONObject: dictionary)
            if let json = String(data:data, encoding:.utf8){
                XCTAssert(!json.contains(","), "Expected result: \"{\"decimalValue\":1.1000000000000001}\" found: \"\(json)\"" )
            }else{
                XCTFail("Failed during string encoding")
            }
        }catch{
            XCTFail("Failed during serialization")
        }
    }

The simple CLI code that proves the bug (https://bugs.swift.org/browse/SR-6631\)

//
// main.swift
// doubleEncoding
//
// Created by Benoit Pereira da silva on 02/12/2017.
// Copyright © 2017 Pereira da Silva. All rights reserved.
//

import Foundation

// Let's set to french
setlocale(LC_ALL,"fr_FR")

do{
    let dictionary = ["decimalValue":1.1]
    let data = try JSONSerialization.data(withJSONObject: dictionary)
    if let json = String(data:data, encoding:.utf8){
        // the result is : "{"decimalValue":1,1000000000000001}"
        // The decimal separator should not be "," but "."
        print(json)
    }
}catch{
    print("\(error)")
}

Any idea ?
I do use Toolchain: Swift Development Snapshot 2017-12-17 (a)

Benoit

Benoit Pereira da Silva
Ultra Mobile Developer & Movement Activist
Développeur Ultra Mobile & Militant du mouvement
https://pereira-da-silva.com <https://pereira-da-silva.com/&gt;

✄ --------------------------------
This e-mail is confidential. Distribution, copy, publication or use of this information for any purpose is prohibited without agreement of the sender.
Ce message est confidentiel. Toute distribution, copie, publication ou usage des informations contenues dans ce message sont interdits sans agrément préalable de l'expéditeur.

Good find,

You can make a pull request and we can get our continuous integration servers to start building that and testing your change – that is probably the easiest way to get validation on your tests and changes.

There are a few issues however with your test that might be worth considering. Comments inline...

Dear All,

I do encounter a very strange Issue.

I have been adding a test to `swift-corelibs-foundation/testJSONSerialization.swift`
This test succeed within swift-corelibs-foundation test, but fails when repackaged to a Cocoa CLI app.

The swift-corelibs-foundation test :

    func test_serializeFloatingPointWithANonDotDecimalSeparatorLocale() {
        // We use a locale that is using "," as decimal separator.
        setlocale(LC_ALL,"fr_FR”)

This change will set the locale globally for the rest of the process, you probably want to make sure to reset the locale back to it’s original state.

        let dictionary = ["decimalValue":1.1]
        do{
            let data = try JSONSerialization.data(withJSONObject: dictionary)
            if let json = String(data:data, encoding:.utf8){
                XCTAssert(!json.contains(","), "Expected result: \"{\"decimalValue\":1.1000000000000001}\" found: \"\(json)\"" )
            }else{
                XCTFail("Failed during string encoding")
            }
        }catch{
            XCTFail("Failed during serialization")
        }
    }

The simple CLI code that proves the bug (https://bugs.swift.org/browse/SR-6631\)

//
// main.swift
// doubleEncoding
//
// Created by Benoit Pereira da silva on 02/12/2017.
// Copyright © 2017 Pereira da Silva. All rights reserved.
//

import Foundation

// Let's set to french
setlocale(LC_ALL,"fr_FR")

do{
    let dictionary = ["decimalValue":1.1]
    let data = try JSONSerialization.data(withJSONObject: dictionary)
    if let json = String(data:data, encoding:.utf8){
        // the result is : "{"decimalValue":1,1000000000000001}"
        // The decimal separator should not be "," but "."
        print(json)
    }
}catch{
    print("\(error)")
}

Any idea ?
I do use Toolchain: Swift Development Snapshot 2017-12-17 (a)

Building a CLI tool will use the objective-c Foundation in the system so that wont use your freshly built swift-corelibs-foundation. So that means we have a bug in the objc side if this is actually happening on Darwin (which is a completely different issue…) That should be a radar against Foundation and I definitely think that may very well be a bug…

···

On Dec 18, 2017, at 8:45 AM, Benoit Pereira da silva via swift-corelibs-dev <swift-corelibs-dev@swift.org> wrote:

Benoit

Benoit Pereira da Silva
Ultra Mobile Developer & Movement Activist
Développeur Ultra Mobile & Militant du mouvement
https://pereira-da-silva.com <https://pereira-da-silva.com/&gt;

<bannerp.jpg>

✄ --------------------------------
This e-mail is confidential. Distribution, copy, publication or use of this information for any purpose is prohibited without agreement of the sender.
Ce message est confidentiel. Toute distribution, copie, publication ou usage des informations contenues dans ce message sont interdits sans agrément préalable de l'expéditeur.

_______________________________________________
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev

Thanks Philippe,

Good find,
You can make a pull request and we can get our continuous integration servers to start building that and testing your change – that is probably the easiest way to get validation on your tests and changes.
There are a few issues however with your test that might be worth considering. Comments inline…

Building a CLI tool will use the objective-c Foundation in the system so that wont use your freshly built swift-corelibs-foundation. So that means we have a bug in the objc side if this is actually happening on Darwin (which is a completely different issue…) That should be a radar against Foundation and I definitely think that may very well be a bug…

I have been filling a radar Feedback Assistant <Feedback Assistant;
I do think you are right it's a bug in the Objc Foundation… and trying to solve it in "swift-corelibs-foundation" was a mistake :)

        setlocale(LC_ALL,"fr_FR”)

This change will set the locale globally for the rest of the process, you probably want to make sure to reset the locale back to it’s original state.

Setting the locale globally was just a temporary test.

But i do think that it triggers a serious question: Should all your tests be ran on all the available locales?
You will certainly find a smarter solution …

Benoit

Thanks Philippe,

Good find,
You can make a pull request and we can get our continuous integration servers to start building that and testing your change – that is probably the easiest way to get validation on your tests and changes.
There are a few issues however with your test that might be worth considering. Comments inline…

Building a CLI tool will use the objective-c Foundation in the system so that wont use your freshly built swift-corelibs-foundation. So that means we have a bug in the objc side if this is actually happening on Darwin (which is a completely different issue…) That should be a radar against Foundation and I definitely think that may very well be a bug…

I have been filling a radar Feedback Assistant <Feedback Assistant;

Thanks for taking the time to file that! I will make sure that gets to the right owner.

I do think you are right it's a bug in the Objc Foundation… and trying to solve it in "swift-corelibs-foundation" was a mistake :)

        setlocale(LC_ALL,"fr_FR”)

This change will set the locale globally for the rest of the process, you probably want to make sure to reset the locale back to it’s original state.

Setting the locale globally was just a temporary test.

But i do think that it triggers a serious question: Should all your tests be ran on all the available locales?
You will certainly find a smarter solution …

I think it is sensible to iterate through a key list of known locales that have certain characteristics, such as using the , as a decimal separator or « for quote begin etc (not to pick on French but it is the one I know better than other punctuation differentials from English).

There are probably key areas that are worth doing this to and others that probably do not matter so much. e.g. you don’t really need to test locale variations with NotificationCenter for example whereas NumberFormatter or JSONSerialization may be places that we want to test a few locales by subclassing the unit tests and in setup change the locale and teardown reset it.

···

On Dec 18, 2017, at 9:29 AM, Benoit Pereira da silva <bpds@me.com> wrote:

Benoit

But i do think that it triggers a serious question: Should all your tests be ran on all the available locales?
You will certainly find a smarter solution …

I think it is sensible to iterate through a key list of known locales that have certain characteristics, such as using the , as a decimal separator or « for quote begin etc (not to pick on French but it is the one I know better than other punctuation differentials from English).

There are probably key areas that are worth doing this to and others that probably do not matter so much. e.g. you don’t really need to test locale variations with NotificationCenter for example whereas NumberFormatter or JSONSerialization may be places that we want to test a few locales by subclassing the unit tests and in setup change the locale and teardown reset it.

Philippe,

Being subtile is not always the best approach.
Are we really able to determinate where such a problem may occur? Should we even try?
I’m not sure !

Smarts approaches may lead to enabling a root account with a blank password.
Running tests on any locale on every commit is absurd.

But running it before a major release, may enable to detect blind spots.

B