|
| 1 | +# Dedicated `.bug()` functions for URLs and IDs |
| 2 | + |
| 3 | +* Proposal: [SWT-0001](0001-refactor-bug-inits.md) |
| 4 | +* Authors: [Jonathan Grynspan](https://github.com/grynspan) |
| 5 | +* Status: **Accepted** |
| 6 | +* Implementation: [apple/swift-testing#401](https://github.com/apple/swift-testing/pull/401) |
| 7 | +* Review: ([pitch](https://forums.swift.org/t/pitch-dedicated-bug-functions-for-urls-and-ids/71842)), ([acceptance](https://forums.swift.org/t/swt-0001-dedicated-bug-functions-for-urls-and-ids/71842/2)) |
| 8 | + |
| 9 | +## Introduction |
| 10 | + |
| 11 | +One of the features of swift-testing is a test traits system that allows |
| 12 | +associating metadata with a test suite or test function. One trait in |
| 13 | +particular, `.bug()`, has the potential for integration with development tools |
| 14 | +but needs some refinement before integration would be practical. |
| 15 | + |
| 16 | +## Motivation |
| 17 | + |
| 18 | +A test author can associate a bug (AKA issue, problem, ticket, etc.) with a test |
| 19 | +using the `.bug()` trait, to which they pass an "identifier" for the bug. The |
| 20 | +swift-testing team's intent here was that a test author would pass the unique |
| 21 | +identifier of the bug in the test author's preferred bug-tracking system (e.g. |
| 22 | +GitHub Issues, Bugzilla, etc.) and that any tooling built around this trait |
| 23 | +would be able to infer where the bug was located and how to view it. |
| 24 | + |
| 25 | +It became clear immediately that a generic system for looking up bugs by unique |
| 26 | +identifier in an arbitrary and unspecified database wouldn't be a workable |
| 27 | +solution. So we modified the description of `.bug()` to explain that, if the |
| 28 | +identifier passed to it was a valid URL, then it would be "interpreted" as a URL |
| 29 | +and that tools could be designed to open that URL as needed. |
| 30 | + |
| 31 | +This design change then placed the burden of parsing each `.bug()` trait and |
| 32 | +potentially mapping it to a URL on tools. swift-testing itself avoids linking to |
| 33 | +or using Foundation API such as `URL`, so checking for a valid URL inside the |
| 34 | +testing library was not feasible either. |
| 35 | + |
| 36 | +## Proposed solution |
| 37 | + |
| 38 | +To solve the underlying problem and allow test authors to specify a URL when |
| 39 | +available, or just an opaque identifier otherwise, we propose splitting the |
| 40 | +`.bug()` function up into two overloads: |
| 41 | + |
| 42 | +- The first overload takes a URL string and additional optional metadata; |
| 43 | +- The second overload takes a bug identifier as an opaque string or integer and, |
| 44 | + optionally, a URL string. |
| 45 | + |
| 46 | +Test authors are then free to specify any combination of URL and opaque |
| 47 | +identifier depending on the information they have available and their specific |
| 48 | +needs. Tools authors are free to consume either or both of these properties and |
| 49 | +present them where appropriate. |
| 50 | + |
| 51 | +## Detailed design |
| 52 | + |
| 53 | +The `Bug` trait type and `.bug()` trait factory function shall be refactored |
| 54 | +thusly: |
| 55 | + |
| 56 | +```swift |
| 57 | +/// A type representing a bug report tracked by a test. |
| 58 | +/// |
| 59 | +/// To add this trait to a test, use one of the following functions: |
| 60 | +/// |
| 61 | +/// - ``Trait/bug(_:_:)`` |
| 62 | +/// - ``Trait/bug(_:id:_:)-10yf5`` |
| 63 | +/// - ``Trait/bug(_:id:_:)-3vtpl`` |
| 64 | +public struct Bug: TestTrait, SuiteTrait, Equatable, Hashable, Codable { |
| 65 | + /// A URL linking to more information about the bug, if available. |
| 66 | + /// |
| 67 | + /// The value of this property represents a URL conforming to |
| 68 | + /// [RFC 3986](https://www.ietf.org/rfc/rfc3986.txt). |
| 69 | + public var url: String? |
| 70 | + |
| 71 | + /// A unique identifier in this bug's associated bug-tracking system, if |
| 72 | + /// available. |
| 73 | + /// |
| 74 | + /// For more information on how the testing library interprets bug |
| 75 | + /// identifiers, see <doc:BugIdentifiers>. |
| 76 | + public var id: String? |
| 77 | + |
| 78 | + /// The human-readable title of the bug, if specified by the test author. |
| 79 | + public var title: Comment? |
| 80 | +} |
| 81 | + |
| 82 | +extension Trait where Self == Bug { |
| 83 | + /// Construct a bug to track with a test. |
| 84 | + /// |
| 85 | + /// - Parameters: |
| 86 | + /// - url: A URL referring to this bug in the associated bug-tracking |
| 87 | + /// system. |
| 88 | + /// - title: Optionally, the human-readable title of the bug. |
| 89 | + /// |
| 90 | + /// - Returns: An instance of ``Bug`` representing the specified bug. |
| 91 | + public static func bug(_ url: _const String, _ title: Comment? = nil) -> Self |
| 92 | + |
| 93 | + /// Construct a bug to track with a test. |
| 94 | + /// |
| 95 | + /// - Parameters: |
| 96 | + /// - url: A URL referring to this bug in the associated bug-tracking |
| 97 | + /// system. |
| 98 | + /// - id: The unique identifier of this bug in its associated bug-tracking |
| 99 | + /// system. |
| 100 | + /// - title: Optionally, the human-readable title of the bug. |
| 101 | + /// |
| 102 | + /// - Returns: An instance of ``Bug`` representing the specified bug. |
| 103 | + public static func bug(_ url: _const String? = nil, id: some Numeric, _ title: Comment? = nil) -> Self |
| 104 | + |
| 105 | + /// Construct a bug to track with a test. |
| 106 | + /// |
| 107 | + /// - Parameters: |
| 108 | + /// - url: A URL referring to this bug in the associated bug-tracking |
| 109 | + /// system. |
| 110 | + /// - id: The unique identifier of this bug in its associated bug-tracking |
| 111 | + /// system. |
| 112 | + /// - title: Optionally, the human-readable title of the bug. |
| 113 | + /// |
| 114 | + /// - Returns: An instance of ``Bug`` representing the specified bug. |
| 115 | + public static func bug(_ url: _const String? = nil, id: _const String, _ title: Comment? = nil) -> Self |
| 116 | +} |
| 117 | +``` |
| 118 | + |
| 119 | +The `@Test` and `@Suite` macros have already been modified so that they perform |
| 120 | +basic validation of a URL string passed as input and emit a diagnostic if the |
| 121 | +URL string appears malformed. |
| 122 | + |
| 123 | +## Source compatibility |
| 124 | + |
| 125 | +This change is expected to be source-breaking for test authors who have already |
| 126 | +adopted the existing `.bug()` functions. This change is source-breaking for code |
| 127 | +that directly refers to these functions by their signatures. This change is |
| 128 | +source-breaking for code that uses the `identifier` property of the `Bug` type |
| 129 | +or expects it to contain a URL. |
| 130 | + |
| 131 | +## Integration with supporting tools |
| 132 | + |
| 133 | +Tools that integrate with swift-testing and provide lists of tests or record |
| 134 | +results after tests have run can use the `Bug` trait on tests to present |
| 135 | +relevant identifiers and/or URLs to users. |
| 136 | + |
| 137 | +Tools that use the experimental event stream output feature of the testing |
| 138 | +library will need a JSON schema for bug traits on tests. This work is tracked in |
| 139 | +a separate upcoming proposal. |
| 140 | + |
| 141 | +## Alternatives considered |
| 142 | + |
| 143 | +- Inferring whether or not a bug identifier was a URL by parsing it at runtime |
| 144 | + in tools. As discussed above, this option would require every tool that |
| 145 | + integrates with swift-testing to provide its own URL-parsing logic. |
| 146 | + |
| 147 | +- Using different argument labels (e.g. the label `url` for the URL argument |
| 148 | + and/or no label for the `id` argument.) We felt that URLs, which are |
| 149 | + recognizable by their general structure, did not need labels. At least one |
| 150 | + argument must have a label to avoid ambiguous resolution of the `.bug()` |
| 151 | + function at compile time. |
| 152 | + |
| 153 | +- Inferring whether or not a bug identifier was a URL by parsing it at compile- |
| 154 | + time or at runtime using `Foundation.URL` or libcurl. swift-testing actively |
| 155 | + avoids linking to Foundation if at all possible, and libcurl would be a |
| 156 | + platform-specific solution (Windows doesn't ship with libcurl, but does have |
| 157 | + `InternetCrackUrlW()` whose parsing engine differs.) We also run the risk of |
| 158 | + inappropriately interpreting some arbitrary bug identifier as a URL when it is |
| 159 | + not meant to be parsed that way. |
| 160 | + |
| 161 | +- Removing the `.bug()` trait. We see this particular trait as having strong |
| 162 | + potential for integration with tools and for use by test authors; removing it |
| 163 | + because we can't reliably parse URLs would be unfortunate. |
| 164 | + |
| 165 | +## Acknowledgments |
| 166 | + |
| 167 | +Thanks to the swift-testing team and managers for their contributions! Thanks to |
| 168 | +our community for the initial feedback around this feature. |
0 commit comments