Fix the code points of ≪⃒ and add a test case (#67) #68
Conversation
kkebo
changed the title
|
ping |
| let url = URL(string: "https://html.spec.whatwg.org/entities.json")! | ||
| URLSession.shared.dataTask(with: url) { data, _, error in | ||
| if let error = error { | ||
| dataTaskError = error | ||
| } else if let data = data { | ||
| entitiesData = data | ||
| } | ||
| expectation.fulfill() | ||
| }.resume() | ||
|
|
||
| self.wait(for: [expectation], timeout: 60) |
There was a problem hiding this comment.
Nice touch adding a test against a known reference. I'm not sure it's a good idea to hit the server on every unit test, though. There could be other problems (network issues, unexpected changes upstream etc.). Perhaps it'd be better to keep this in the repo?
At 140k raw, it's not "huge". Assuming it's not something that changes often, it's probably fine to keep in the repo.
There was a problem hiding this comment.
Thank you for reviewing.
Your concerns are reasonable. I also thought about it at first. However, I realized that I would have to verify that the file was correct. It seems to me that using a local file to validate *DecodeMaps simply postpones the problem, since another way is needed to validate the file itself.
Moreover, since the HTML specs keep changing, there will come a time when the file will have to be changed. We must then verify in some way that those changes are correct.
What do you think?
There was a problem hiding this comment.
That's actually added reason to keep a copy of the file in the repo so that it can be manually inspected each time. If it's automated, then the test can break at any time and it'll be unclear why.
There was a problem hiding this comment.
Thanks for replying. The main point I wrote was that it didn't make sense to add a file to the repo because I thought verifying it wasn't much different than verifying Swift constants. But it's a copy of a file published as a part of the specs, so it is easier to verify than Swift constants. I changed my mind. I'm going to add the file and rewrite the test.
There was a problem hiding this comment.
I pushed the commit b86aa37.
There was a problem hiding this comment.
@dannys42 I am not in a hurry, but what do you think?
|
Kudos, SonarCloud Quality Gate passed!
|
|
ping |
2 participants
Description
fixes #67
I fixed the code points of ≪⃒ (
≪⃒) innamedCharactersDecodeMap2and added a test case to verify if*DecodeMaps match https://html.spec.whatwg.org/entities.json.Motivation and Context
#67 and #65 (comment)
How Has This Been Tested?
I added a test case of
*DecodeMaps, so I tested this modification by it.Checklist: