Skip to content

Commit

Permalink
[NFC] Refactor PetstoreConsumerTests to allow multiple versions (#157)
Browse files Browse the repository at this point in the history
[NFC] Refactor PetstoreConsumerTests to allow multiple versions

### Motivation

In upcoming features that contain breaking changes, but we'll stage them in behind a feature flag, we'd like to have multiple variants of the `PetstoreConsumerTests` test target, allowing us to test both the existing behavior and the new behavior once a feature flag is enabled. These additional variants would be temporary, and would be deleted again (or rather, the main test suite would be updated) once the feature flag is enabled unconditionally. So the steady state number of `PetstoreConsumerTest` variants would continue to be 1, just for short periods of time, it might be higher.

### Modifications

This PR makes that possible by refactoring common functionality into a new internal target `PetstoreConsumerTestCore`. Note that all the variants of the test target share the same OpenAPI document, but generate different variants of the reference code.

Highlights:
- new `PetstoreConsumerTestCore` target, depended on by the existing `PetstoreConsumerTests`
- added an ability to _not_ fail the test when specific diagnostics are emitted (through the new `ignoredDiagnosticMessages: Set<String>` parameter), which allows us to test both existing and new behavior on the same OpenAPI document. Other, non-allowlisted diagnostics, still continue to fail the test, so new ones won't sneak through undetected.
- many test functions now optionally take `featureFlags` and `ignoredDiagnosticMessages`, again allowing us to test both existing and proposed behavior hidden behind a feature flag

### Result

It will be a lot easier to temporarily introduce other variants of the `PetstoreConsumerTests` test target to allow for thoroughly testing features even before they are enabled by default, giving us more confidence in the accuracy of proposals and their associated implementations.

### Test Plan

All tests continue to pass, this is an NFC, a pure refactoring, making the next PR much smaller. To see how this all fits together, check out the PR that has all the changes: #146. That said, these partial PRs are easier to review, as they're each focused on one task.


Reviewed by: gjcairo, simonjbeaumont

Builds:
     ✔︎ pull request validation (5.8) - Build finished. 
     ✔︎ pull request validation (5.9) - Build finished. 
     ✔︎ pull request validation (docc test) - Build finished. 
     ✔︎ pull request validation (integration test) - Build finished. 
     ✔︎ pull request validation (nightly) - Build finished. 
     ✔︎ pull request validation (soundness) - Build finished. 

#157
  • Loading branch information
czechboy0 authored Aug 1, 2023
1 parent b1d310f commit 379b047
Show file tree
Hide file tree
Showing 13 changed files with 209 additions and 136 deletions.
11 changes: 10 additions & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,22 @@ let package = Package(
swiftSettings: swiftSettings
),

// Common types for concrete PetstoreConsumer*Tests test targets.
.target(
name: "PetstoreConsumerTestCore",
dependencies: [
.product(name: "OpenAPIRuntime", package: "swift-openapi-runtime")
],
swiftSettings: swiftSettings
),

// PetstoreConsumerTests
// Builds and tests the reference code from GeneratorReferenceTests
// to ensure it actually works correctly at runtime.
.testTarget(
name: "PetstoreConsumerTests",
dependencies: [
.product(name: "OpenAPIRuntime", package: "swift-openapi-runtime")
"PetstoreConsumerTestCore"
],
swiftSettings: swiftSettings
),
Expand Down
131 changes: 131 additions & 0 deletions Sources/PetstoreConsumerTestCore/Common.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftOpenAPIGenerator open source project
//
// Copyright (c) 2023 Apple Inc. and the SwiftOpenAPIGenerator project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftOpenAPIGenerator project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
import OpenAPIRuntime
import Foundation

public enum TestError: Swift.Error, LocalizedError, CustomStringConvertible {
case noHandlerFound(method: HTTPMethod, path: [RouterPathComponent])
case invalidURLString(String)
case unexpectedValue(Any)
case unexpectedMissingRequestBody

public var description: String {
switch self {
case .noHandlerFound(let method, let path):
return "No handler found for method \(method.name) and path \(path.stringPath)"
case .invalidURLString(let string):
return "Invalid URL string: \(string)"
case .unexpectedValue(let value):
return "Unexpected value: \(value)"
case .unexpectedMissingRequestBody:
return "Unexpected missing request body"
}
}

public var errorDescription: String? {
description
}
}

public extension Date {
static var test: Date {
Date(timeIntervalSince1970: 1_674_036_251)
}

static var testString: String {
"2023-01-18T10:04:11Z"
}
}

public extension Array where Element == RouterPathComponent {
var stringPath: String {
map(\.description).joined(separator: "/")
}
}

public extension Response {
init(
statusCode: Int,
headers: [HeaderField] = [],
encodedBody: String
) {
self.init(
statusCode: statusCode,
headerFields: headers,
body: Data(encodedBody.utf8)
)
}

static var listPetsSuccess: Self {
.init(
statusCode: 200,
headers: [
.init(name: "content-type", value: "application/json")
],
encodedBody: #"""
[
{
"id": 1,
"name": "Fluffz"
}
]
"""#
)
}
}

public extension Data {
var pretty: String {
String(decoding: self, as: UTF8.self)
}

static var abcdString: String {
"abcd"
}

static var abcd: Data {
Data(abcdString.utf8)
}

static var efghString: String {
"efgh"
}

static var quotedEfghString: String {
#""efgh""#
}

static var efgh: Data {
Data(efghString.utf8)
}
}

public extension Request {
init(
path: String,
query: String? = nil,
method: HTTPMethod,
headerFields: [HeaderField] = [],
encodedBody: String
) throws {
let body = Data(encodedBody.utf8)
self.init(
path: path,
query: query,
method: method,
headerFields: headerFields,
body: body
)
}
}
50 changes: 38 additions & 12 deletions Tests/OpenAPIGeneratorReferenceTests/FileBasedReferenceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class FileBasedReferenceTests: XCTestCase {
}

func testPetstore() throws {
try _test(referenceProjectNamed: .petstore)
try _test(referenceProject: .init(name: .petstore))
}

// MARK: - Private
Expand All @@ -60,7 +60,8 @@ class FileBasedReferenceTests: XCTestCase {
}

func performReferenceTest(
_ referenceTest: TestConfig
_ referenceTest: TestConfig,
ignoredDiagnosticMessages: Set<String> = []
) throws {
print(
"""
Expand All @@ -85,7 +86,8 @@ class FileBasedReferenceTests: XCTestCase {

// Run the requested generator invocation
let generatorPipeline = self.makeGeneratorPipeline(
config: referenceTest.asConfig
config: referenceTest.asConfig,
ignoredDiagnosticMessages: ignoredDiagnosticMessages
)
let generatedOutputSource = try generatorPipeline.run(input)

Expand Down Expand Up @@ -115,16 +117,33 @@ class FileBasedReferenceTests: XCTestCase {
enum ReferenceProjectName: String, Hashable, CaseIterable {
case petstore

var fileName: String {
var openAPIDocFileName: String {
"\(rawValue).yaml"
}

var directoryName: String {
var fixtureCodeDirectoryName: String {
rawValue.capitalized
}
}

func _test(referenceProjectNamed name: ReferenceProjectName) throws {
struct ReferenceProject: Hashable, Equatable {
var name: ReferenceProjectName
var customDirectoryName: String? = nil

var fixtureCodeDirectoryName: String {
customDirectoryName ?? name.fixtureCodeDirectoryName
}

var openAPIDocFileName: String {
name.openAPIDocFileName
}
}

func _test(
referenceProject project: ReferenceProject,
featureFlags: FeatureFlags = [],
ignoredDiagnosticMessages: Set<String> = []
) throws {
let modes: [GeneratorMode] = [
.types,
.client,
Expand All @@ -133,19 +152,23 @@ class FileBasedReferenceTests: XCTestCase {
for mode in modes {
try performReferenceTest(
.init(
docFilePath: "Docs/\(name.fileName)",
docFilePath: "Docs/\(project.openAPIDocFileName)",
mode: mode,
additionalImports: [],
featureFlags: [],
referenceOutputDirectory: "ReferenceSources/\(name.directoryName)"
)
featureFlags: featureFlags,
referenceOutputDirectory: "ReferenceSources/\(project.fixtureCodeDirectoryName)"
),
ignoredDiagnosticMessages: ignoredDiagnosticMessages
)
}
}
}

extension FileBasedReferenceTests {
private func makeGeneratorPipeline(config: Config) -> GeneratorPipeline {
private func makeGeneratorPipeline(
config: Config,
ignoredDiagnosticMessages: Set<String> = []
) -> GeneratorPipeline {
let parser = YamsParser()
let translator = MultiplexTranslator()
let renderer = TextBasedRenderer()
Expand All @@ -160,7 +183,10 @@ extension FileBasedReferenceTests {
return newFile
},
config: config,
diagnostics: XCTestDiagnosticCollector(test: self)
diagnostics: XCTestDiagnosticCollector(
test: self,
ignoredDiagnosticMessages: ignoredDiagnosticMessages
)
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,11 +605,15 @@ extension SnippetBasedReferenceTests {
)
}

func makeTypesTranslator(componentsYAML: String) throws -> TypesFileTranslator {
func makeTypesTranslator(
featureFlags: FeatureFlags = [],
ignoredDiagnosticMessages: Set<String> = [],
componentsYAML: String
) throws -> TypesFileTranslator {
let components = try YAMLDecoder().decode(OpenAPI.Components.self, from: componentsYAML)
return TypesFileTranslator(
config: Config(mode: .types),
diagnostics: XCTestDiagnosticCollector(test: self),
config: Config(mode: .types, featureFlags: featureFlags),
diagnostics: XCTestDiagnosticCollector(test: self, ignoredDiagnosticMessages: ignoredDiagnosticMessages),
components: components
)
}
Expand Down Expand Up @@ -648,23 +652,35 @@ extension SnippetBasedReferenceTests {
}

func assertResponsesTranslation(
featureFlags: FeatureFlags = [],
ignoredDiagnosticMessages: Set<String> = [],
_ componentsYAML: String,
_ expectedSwift: String,
file: StaticString = #filePath,
line: UInt = #line
) throws {
let translator = try makeTypesTranslator(componentsYAML: componentsYAML)
let translator = try makeTypesTranslator(
featureFlags: featureFlags,
ignoredDiagnosticMessages: ignoredDiagnosticMessages,
componentsYAML: componentsYAML
)
let translation = try translator.translateComponentResponses(translator.components.responses)
try XCTAssertSwiftEquivalent(translation, expectedSwift, file: file, line: line)
}

func assertRequestBodiesTranslation(
featureFlags: FeatureFlags = [],
ignoredDiagnosticMessages: Set<String> = [],
_ componentsYAML: String,
_ expectedSwift: String,
file: StaticString = #filePath,
line: UInt = #line
) throws {
let translator = try makeTypesTranslator(componentsYAML: componentsYAML)
let translator = try makeTypesTranslator(
featureFlags: featureFlags,
ignoredDiagnosticMessages: ignoredDiagnosticMessages,
componentsYAML: componentsYAML
)
let translation = try translator.translateComponentRequestBodies(translator.components.requestBodies)
try XCTAssertSwiftEquivalent(translation, expectedSwift, file: file, line: line)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@ import XCTest
// A diagnostic collector that fails the running test on a warning or error.
struct XCTestDiagnosticCollector: DiagnosticCollector {
var test: XCTestCase
var ignoredDiagnosticMessages: Set<String> = []

func emit(_ diagnostic: Diagnostic) {
guard !ignoredDiagnosticMessages.contains(diagnostic.message) else {
return
}
print("Test emitted diagnostic: \(diagnostic.description)")
switch diagnostic.severity {
case .note:
Expand Down
Loading

0 comments on commit 379b047

Please sign in to comment.