search

LiveUI / XMLCoding

XMLEncoder & XMLDecoder using the Codable protocol in Swift 4.2
MIT License
10 stars 8 forks source link

Tests in Linux #5

Closed rafiki270 closed 6 years ago

rafiki270 commented 6 years ago

Not sure why the code doesn't encode stuff properly on Linux

To see the results (if you got docker) try ./scripts/docker-shortcuts/test.sh

Does it makes any sense to you @tachyonics? Also, would you mind getting connected on the the LiveUI slack or Vapor Discord so we don't have to chat through PR's & issues? :)

rafiki270 commented 6 years ago

CI is here https://ci.liveui.io/job/LiveUI/job/XMLCoding/ mac mini will be up again tomorrow as I had hdd replaced for an SSD and the whole thing reformatted so need to setup access to it again

Also, the tests only work sometimes as the <Result /> is sometimes at the bottom of the generated xml and sometimes at the beginning :( ... maybe we should separate the encode/decode tests somehow

tachyonics commented 6 years ago

It looks like the main problem is caused by the different behavior in bridging with the NS* types on Linux vs Mac. This is causing the serializer to silently ignore most of the type hierarchy as it expecting an NSDictionary but is getting a [AnyHashable: Any] Swift Dictionary.

I have put some changes on the linuxEncodingTest branch which essentially replaces the use of NS* types with native Swift types. I have also fixed the ordering problem you mentioned in the tests by de-serializing the results back out to types before comparing them. I have confirmed the unit tests now pass using the docker script. Requires some additional testing. Let me know what you think.

rafiki270 commented 6 years ago

haven't had a time to test yet but looks really good in GH ... would you please mind making a PR so we can see what the CI thinks? also, you can now merge PR's and stuff but let's get into the habit of approving each others PR's first

tachyonics commented 6 years ago

Sorry for the delay. I wanted to add some unit tests which I have now created a PR for. Looks like you have merged my previous changes though.