Add tests for /api/v1/locations/{language} endpoint#98
Conversation
This makes tests directly reference the endpoint they are testing, to resolve any ambiguity.
Jonathan-Zollinger
left a comment
There was a problem hiding this comment.
really great work! a few changes, but this would have been a great PR without these changes as well - great work!
| - Image | ||
| - User | ||
| - Organization | ||
| - Project |
There was a problem hiding this comment.
| - Project | |
| - Location | |
| - Locale |
| CountryStatePair: | ||
| type: object | ||
| properties: | ||
| countryInfo: | ||
| $ref: "#/components/schemas/CountryInfo" | ||
| country: | ||
| type: string | ||
| nullable: true | ||
| state: | ||
| type: string | ||
| nullable: true | ||
| county: | ||
| type: string | ||
| nullable: true |
There was a problem hiding this comment.
Let's take this opportunity to improve on this name since it's not really a 'pair'. I'm interested to hear your suggestions, but maybe something along the lines of "CountryContext" as it's more accurate to its name?
| CountryInfo: | ||
| type: object | ||
| properties: | ||
| twoCharCode: | ||
| type: string | ||
| nullable: true | ||
| threeCharCode: | ||
| type: string | ||
| nullable: true | ||
| countryPhone: | ||
| type: string | ||
| nullable: true | ||
| additionalProperties: false |
There was a problem hiding this comment.
another opportunity to have this make more sense than what we originally inherited - let's go with 'CountryIdentifiers'. When I first read "country info", I was thinking I'd see data about the country at a high level, but it's specifically identifiers and nothing more.
| type: object | ||
| properties: | ||
| twoCharCode: | ||
| type: string |
There was a problem hiding this comment.
| type: string | |
| type: string | |
| description: "ISO 3166-1 standard two letter country code" |
| type: string | ||
| nullable: true | ||
| threeCharCode: | ||
| type: string |
There was a problem hiding this comment.
| type: string | |
| type: string | |
| description: "ISO 3166-1 standard three letter country code" |
| get: | ||
| tags: [ Location ] | ||
| description: Get all locations in a language | ||
| operationId: getLanguage |
There was a problem hiding this comment.
| operationId: getLanguage | |
| operationId: getLocation |
| schema: | ||
| type: array | ||
| items: | ||
| $ref: "#/components/schemas/CountryStatePair" |
There was a problem hiding this comment.
see my comments on 2044
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/ImageUploadResponse' | ||
| /api/v1/locations/{language}: |
There was a problem hiding this comment.
Let's also add the GET Languages endpoint for our tests
| /api/v1/locations/{language}: | |
| /api/v1/languages: | |
| get: | |
| tags: | |
| - Language | |
| responses: | |
| "200": | |
| description: OK | |
| content: | |
| text/plain: | |
| schema: | |
| type: array | |
| items: | |
| $ref: "#/components/schemas/LocaleNames" | |
| application/json: | |
| schema: | |
| type: array | |
| items: | |
| $ref: "#/components/schemas/LocaleNames" | |
| text/json: | |
| schema: | |
| type: array | |
| items: | |
| $ref: "#/components/schemas/LocaleNames" | |
| /api/v1/locations/{language}: |
| nullable: true | ||
| county: | ||
| type: string | ||
| nullable: true |
There was a problem hiding this comment.
| nullable: true | |
| nullable: true | |
| LocaleNames: | |
| type: object | |
| properties: | |
| bcp47: | |
| description: locale abbreviation per the <a href='https://developer.mozilla.org/en-US/docs/Glossary/BCP_47_language_tag' BCP 47 standard</a> | |
| type: string | |
| nullable: true | |
| name: | |
| description: locale name | |
| type: string | |
| nullable: true |
for the name field, can you see if it returns the three char code that would map to the field on line 2024 or if it's a human-readable field? if it's meant to be the three char field, let's document it as such.
There was a problem hiding this comment.
Navigating to stage.justserve.org, it appears that the name is a human-readable field. I'll also add the "lang3char" field that you missed.
There was a problem hiding this comment.
check what the response is in stage - the swagger docs I'm using may be outdated but they only reference the two fields I've added above.
There was a problem hiding this comment.
Also, I'm not sure if these are going to be usable, because the lang3char field does not seem to follow any standardized format. For instance, French is "fre", which matches the older legacy code for ISO 639, but Portuguese is "prt", which isn't at all used in ISO 639, but used in ISO 3166 for Portugal. However, "fre" is not present in ISO 3166. So there would have to be some more in-depth restructuring to use these.
You can see what's available by going to /api/languages
There was a problem hiding this comment.
let's remove the iso description from them then, but still add some helpful docs (just without the iso reference). I wish we knew the rhyme or reason for the different strings
| def "result data from .../locations/{language} matches expected format"() { | ||
| given: | ||
| def lang = "eng" | ||
|
|
||
| when: | ||
| def location = locationClient.getLanguage(lang).block().first | ||
|
|
||
| then: | ||
| verifyAll { | ||
| location != null | ||
| location.getCountryInfo() != null | ||
|
|
||
| location.getCountryInfo().getTwoCharCode() == null || location.getCountryInfo().getTwoCharCode() ==~ /[a-z]{2}/ | ||
| location.getCountryInfo().getThreeCharCode() == null || location.getCountryInfo().getThreeCharCode() ==~ /[a-z]{3}/ | ||
| location.getCountryInfo().getCountryPhone() == null || location.getCountryInfo().getCountryPhone() ==~ /\d+/ | ||
| location.getCountry() == null || location.getCountry() ==~ /.+/ | ||
| location.getState() == null || location.getState() ==~ /.+/ | ||
| location.getCounty() == null || location.getCounty() ==~ /.+/ | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
follow the same edits as the test above.
This also adds the needed schema for generated classes in the schema.yml file.