Expand example app for enhanced test coverage#12
Merged
paulsturgess merged 2 commits intomainfrom Nov 16, 2023
Merged
Conversation
paulsturgess
commented
Nov 16, 2023
| schema | ||
|
|
||
| get "example/format", controller: Controllers::TimeController, endpoint: :format | ||
| get "time_formatting/format", controller: Controllers::TimeController, endpoint: :format |
Contributor
Author
There was a problem hiding this comment.
I tweaked this so now we have two endpoints that produce a conflicting id. This is handled in the spec generator, but wasn't executed as part of the test run.
paulsturgess
commented
Nov 16, 2023
Comment on lines
-156
to
+164
| "type": "object", | ||
| "properties": { | ||
| "as_integer": { | ||
| "type": "integer" | ||
| "type": "array", | ||
| "items": { | ||
| "type": "object", | ||
| "properties": { | ||
| "as_integer": { | ||
| "type": "integer" | ||
| } |
Contributor
Author
There was a problem hiding this comment.
this shows how we now accurately describe arrays, in this case an array of objects.
paulsturgess
commented
Nov 16, 2023
Comment on lines
+19
to
+25
| field :as_array_of_strings, type: [:string] do | ||
| backend { |y| y.chars } | ||
| end | ||
|
|
||
| field :as_array_of_enums, type: [Day] do | ||
| backend { %w[Sunday Monday] } | ||
| end |
Contributor
Author
There was a problem hiding this comment.
I've added these to introduce more code paths into the spec run.
paulsturgess
commented
Nov 16, 2023
|
|
||
| description "Returns the current time" | ||
| argument :object, type: ArgumentSets::ObjectLookup, required: true | ||
| argument :scalar, type: :string |
Contributor
Author
There was a problem hiding this comment.
another addition to improve the code coverage in the spec
cc47ebb to
5f2f813
Compare
paulsturgess
commented
Nov 16, 2023
Comment on lines
-73
to
-97
| def build_schema_for_array | ||
| @schema[:type] = "object" | ||
| @schema[:properties] ||= {} | ||
| if @definition.type.argument_set? || @definition.type.enum? || @definition.type.object? | ||
| if @definition.type.argument_set? | ||
| # TODO: add array of argument sets to the example app (refer to CoreAPI::ArgumentSets::KeyValue) | ||
| @children = @definition.type.klass.definition.arguments.values | ||
| else | ||
| @children = @definition.type.klass.definition.fields.values | ||
| end | ||
| else | ||
| items = { type: convert_type_to_open_api_data_type(@definition.type) } | ||
| end | ||
|
|
||
| return unless items | ||
|
|
||
| # TODO: ensure this is triggered by example API | ||
|
|
||
| @schema[:properties][@definition.name.to_s] = { | ||
| type: "array", | ||
| items: items | ||
| } | ||
| @schema | ||
| end | ||
|
|
Contributor
Author
There was a problem hiding this comment.
The schema generation for arrays didn't work for 100% of cases, so I've tweaked it and in the process been able to simplify it which is nice.
5f2f813 to
71b839c
Compare
71b839c to
7843096
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There were a number of codepaths in the spec generator that were developed against the Katapult API but were not represented in the example app contained in this repo. So now all code paths in the generator are triggered.
We can see this, because i've added Simplecov for code coverage checking.
I think this makes sense, as it will ensure that all code-paths used by the specification generator are actually executed by the example application. Which gives us greater confidence that we are generating the spec correctly for all the various ways the apia endpoints can be defined.
This also revealed a bug in the way arrays were represented for some parts of the schema. So this PR also fixes that at the same time.
I've removed a bunch of code comment TODOs because we have issues to track these now.
I have verified these changes against the Katapult API.
closes: #8