corrects schema version property name in RenderIndex.spec.json #1224
Conversation
|
@swift-ci please test |
…lang#1214) This change prevents a potential DocC crash by making sure that it does not access an array element using an index that is outside of the bonds of the array. `.index(after:` can crash in some cases if the passed index belongs to the last element of the collection. with this change we ensure that the passed index is not the last index. rdar://150760264
| @@ -140,6 +141,7 @@ if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil { | |||
| .package(url: "https://github.com/swiftlang/swift-docc-symbolkit.git", branch: "main"), | |||
| .package(url: "https://github.com/apple/swift-crypto.git", from: "3.0.0"), | |||
| .package(url: "https://github.com/swiftlang/swift-docc-plugin.git", from: "1.2.0"), | |||
| .package(url: "https://github.com/apple/swift-openapi-generator", from: "1.8.0"), | |||
There was a problem hiding this comment.
FYI: If we add this dependency, it—and all its transient dependencies—will be used when building DocC for toolchains builds. This significantly increases the amount of work necessary to merge this PR because we need to check if anything else in the toolchain depend on any version of these dependencies and ensure that those versions are compatible, now and in the future. If we can skip this dependency it would make it easier to merge this PR. Otherwise someone will write access to various Swift repositories will need to spend some time making toolchain builds to verify that these new dependencies don't break the toolchain builds.
There was a problem hiding this comment.
yeah, I was afraid of that.
Since the project chose to provide OpenAPI schema files as a description, what would be the best way to verify them? The dependency isn't the only problem - going from them as resources to something you can write a test to verify is another awkward part here.
I wanted to explore what might be possible in testing, and this PR reflects a baseline of it, but it adds a huge amount of overhead, as you're mentioning.
Bug/issue #, if applicable: #1223
Summary
I grabbed the SlothCreator sample project and rendered it using swift-docc, reviewing the JSON it generated to see what might be correct for issue #1223 - and it seems that the correct propertyName is
schemaVersion. This makes that change directly, but there's no existing tests to validate that I spotted, including even validating the the provided OpenAPI spec files are able to be validated.Dependencies
added a dependency on OpenAPIGenerator for the testing scenario
Testing
I've dropped in a test that runs the OpenAPI generator to parse and warn of inconsistencies (as thrown Diagnostics) from an existing spec, which illustrates the failure with the current file, and is resolved when it's updated. However, I could use some advice on how to manage the testing scenario in this case, as I want to explicitly test the resources in the DocC catalog, which aren't generally available to the testing through Bundles.
I've copied the one instance of a spec (RenderIndex.spec) into the test fixtures, to show the test validates the OpenAPI schema spec. The test is currently failing, as expected - but with the spec updated to match the first commit in this PR, it passes.
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded