Skip to content

Expand example app for enhanced test coverage#12

Merged
paulsturgess merged 2 commits intomainfrom
simplecov
Nov 16, 2023
Merged

Expand example app for enhanced test coverage#12
paulsturgess merged 2 commits intomainfrom
simplecov

Conversation

@paulsturgess
Copy link
Copy Markdown
Contributor

@paulsturgess paulsturgess commented Nov 16, 2023

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

Screenshot 2023-11-16 at 09 59 35

schema

get "example/format", controller: Controllers::TimeController, endpoint: :format
get "time_formatting/format", controller: Controllers::TimeController, endpoint: :format
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines -156 to +164
"type": "object",
"properties": {
"as_integer": {
"type": "integer"
"type": "array",
"items": {
"type": "object",
"properties": {
"as_integer": {
"type": "integer"
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shows how we now accurately describe arrays, in this case an array of objects.

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
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added these to introduce more code paths into the spec run.


description "Returns the current time"
argument :object, type: ArgumentSets::ObjectLookup, required: true
argument :scalar, type: :string
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another addition to improve the code coverage in the spec

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@paulsturgess paulsturgess changed the title Simplecov Expand example app for enhanced test coverage Nov 16, 2023
@paulsturgess paulsturgess marked this pull request as ready for review November 16, 2023 10:05
@paulsturgess paulsturgess self-assigned this Nov 16, 2023
@paulsturgess paulsturgess merged commit d74007c into main Nov 16, 2023
@paulsturgess paulsturgess deleted the simplecov branch November 16, 2023 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expand example app to ensure all code-paths are triggered in the generation of the schema.

1 participant