Miscellaneous test changes to avoid making assumptions about what's a…

… valid URL (#586) * Avoid assumption about order of encoded JSON in test * Remove test that verifies what characters redirect path support (This is verifying Foundation behavior that's not redirect specific) * Simplify test that only verify validation of one URL string * Require that source repository base URLs include scheme and host * Avoid testing curation of headings (this isn't supported) * Update test URL to use characters that DocC will replace with "-" * Only print the preview URL if it's non-nil (In practice the port is validated before this but some tests skip that) * Update Swift-Markdown to pick up a index-out-of-bounds fix * Update test URL to use other non-path-allows characters * Revert "Update test URL to use characters that DocC will replace with "-"" 7d1f3bd rdar://108237260
swiftlang · Jul 26, 2023 · 6478439 · 6478439
1 parent 430262e
commit 6478439
Show file tree

Hide file tree

Showing 9 changed files with 40 additions and 55 deletions.
diff --git a/Package.resolved b/Package.resolved
@@ -78,7 +78,7 @@
       "location" : "https://github.com/apple/swift-markdown.git",
       "state" : {
         "branch" : "main",
-        "revision" : "36b71b380ca9cb7497fc24416f8b77721eaf7330"
+        "revision" : "3d4b36cff09f785adf5efb190d458a3d44e6df87"
       }
     },
     {

diff --git a/Sources/SwiftDocCUtilities/Action/Actions/PreviewAction.swift b/Sources/SwiftDocCUtilities/Action/Actions/PreviewAction.swift
@@ -163,9 +163,11 @@ public final class PreviewAction: Action, RecreatingContext {
         // Preview the output and monitor the source bundle for changes.
         do {
             print(String(repeating: "=", count: 40), to: &logHandle)
-            print("Starting Local Preview Server", to: &logHandle)
-            printPreviewAddresses(base: URL(string: "http://localhost:\(port)")!)
-            print(String(repeating: "=", count: 40), to: &logHandle)
+            if let previewURL = URL(string: "http://localhost:\(port)") {
+                print("Starting Local Preview Server", to: &logHandle)
+                printPreviewAddresses(base: previewURL)
+                print(String(repeating: "=", count: 40), to: &logHandle)
+            }
 
             let to: PreviewServer.Bind = bindServerToSocketPath.map { .socket(path: $0) } ?? .localhost(port: port)
             servers[serverIdentifier] = try PreviewServer(contentURL: convertAction.targetDirectory, bindTo: to, logHandle: &logHandle)

diff --git a/...ftDocCUtilities/ArgumentParsing/Options/Source Repository/SourceRepositoryArguments.swift b/...ftDocCUtilities/ArgumentParsing/Options/Source Repository/SourceRepositoryArguments.swift
@@ -78,7 +78,7 @@ extension SourceRepository {
                 """
             )
         case let (sourceService?, sourceServiceBaseURL?, checkoutPath?):
-            guard let sourceServiceBaseURL = URL(string: sourceServiceBaseURL) else {
+            guard let sourceServiceBaseURL = URL(string: sourceServiceBaseURL), sourceServiceBaseURL.scheme != nil, sourceServiceBaseURL.host != nil else {
                 throw ValidationError("Invalid URL '\(sourceServiceBaseURL)' for '--source-service-base-url' argument.")
             }
 

diff --git a/Tests/SwiftDocCTests/Infrastructure/Communication/WebKitCommunicationBridgeTests.swift b/Tests/SwiftDocCTests/Infrastructure/Communication/WebKitCommunicationBridgeTests.swift
@@ -45,15 +45,30 @@ class WebKitCommunicationBridgeTests: XCTestCase {
 
     func assertMessageIsSent(message: Message) throws {
         #if canImport(WebKit)
-        let encodedMessage = try! JSONEncoder().encode(message)
-        let messageJSON = String(data: encodedMessage, encoding: .utf8)!
+        let encodedMessage = try JSONEncoder().encode(message)
+        let json = try XCTUnwrap(JSONSerialization.jsonObject(with: encodedMessage) as? NSDictionary)
 
+        let didEvaluateJavaScript = expectation(description: "Did evaluate JavaScript")
         let evaluateJavaScript: (String, ((Any?, Error?) -> ())?) -> () = { string, _ in
-            XCTAssertEqual(string, "window.bridge.receive(\(messageJSON))")
+            defer { didEvaluateJavaScript.fulfill() }
+
+            XCTAssert(string.hasPrefix("window.bridge.receive("))
+            XCTAssert(string.hasSuffix(")"))
+
+            let jsonString = String(string.dropFirst("window.bridge.receive(".count).dropLast())
+            guard let jsonData = jsonString.data(using: .utf8),
+                  let decodedMessage = try? JSONSerialization.jsonObject(with: jsonData) as? NSDictionary
+            else {
+                XCTFail("Unable to decode \(jsonString) as communication bridge Message")
+                return
+            }
+
+            XCTAssertEqual(json, decodedMessage)
         }
 
         let bridge = WebKitCommunicationBridge()
         XCTAssertNoThrow(try bridge.send(message, using: evaluateJavaScript))
+        waitForExpectations(timeout: 1.0)
         #endif
     }
 }

diff --git a/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift b/Tests/SwiftDocCTests/Infrastructure/DocumentationContext/DocumentationContextTests.swift
@@ -1718,10 +1718,10 @@ let expected = """
             
             ## Topics
             
+            Only curate the pages. Headings don't support curation.
+            
             - ``MyClass/myFunc🙂()``
-            - <doc:article-with-emoji-in-heading#Hello-🌍>
             - <doc:article-with-😃-in-filename>
-            - <doc:article-with-😃-in-filename#Hello-world>
             """),
         ])
         let bundleURL = try testBundle.write(inside: createTemporaryDirectory())
@@ -1739,9 +1739,7 @@ let expected = """
         // Verify that all the links in the topic section resolved
         XCTAssertEqual(topicSection.links.map(\.destination), [
             "doc://special-characters/documentation/MyKit/MyClass/myFunc_()",
-            "doc://special-characters/documentation/special-characters/article-with-emoji-in-heading#Hello-%F0%9F%8C%8D",
             "doc://special-characters/documentation/special-characters/article-with---in-filename",
-            "doc://special-characters/documentation/special-characters/article-with---in-filename#Hello-world",
         ])
 
         // Verify that all resolved link exist in the context.
@@ -1756,12 +1754,10 @@ let expected = """
         let renderNode = translator.visit(moduleSymbol) as! RenderNode
 
         // Verify that the resolved links rendered as links
-        XCTAssertEqual(renderNode.topicSections.first?.identifiers.count, 4)
+        XCTAssertEqual(renderNode.topicSections.first?.identifiers.count, 2)
         XCTAssertEqual(renderNode.topicSections.first?.identifiers, [
             "doc://special-characters/documentation/MyKit/MyClass/myFunc_()",
-            "doc://special-characters/documentation/special-characters/article-with-emoji-in-heading#Hello-%F0%9F%8C%8D",
             "doc://special-characters/documentation/special-characters/article-with---in-filename",
-            "doc://special-characters/documentation/special-characters/article-with---in-filename#Hello-world",
         ])
 
 

diff --git a/Tests/SwiftDocCTests/Infrastructure/ResolvedTopicReferenceTests.swift b/Tests/SwiftDocCTests/Infrastructure/ResolvedTopicReferenceTests.swift
@@ -54,7 +54,12 @@ class ResolvedTopicReferenceTests: XCTestCase {
         do {
             let resolvedOriginal = ResolvedTopicReference(bundleIdentifier: "bundleID", path: "/path/sub-path", fragment: "fragment", sourceLanguage: .swift)
 
-            let unresolved = UnresolvedTopicReference(topicURL: ValidatedURL(parsingExact: "doc://host.name/;;;")!)
+            var components = URLComponents()
+            components.scheme = "doc"
+            components.host = "host.name"
+            components.percentEncodedPath = "%20%20%20" // 3 spaces 
+
+            let unresolved = UnresolvedTopicReference(topicURL: ValidatedURL(components: components))
             XCTAssertFalse(unresolved.path.isEmpty)
 
             let appended = resolvedOriginal.appendingPathOfReference(unresolved)

diff --git a/Tests/SwiftDocCTests/Semantics/RedirectedTests.swift b/Tests/SwiftDocCTests/Semantics/RedirectedTests.swift
@@ -41,29 +41,6 @@ class RedirectedTests: XCTestCase {
         XCTAssertEqual(redirected?.oldPath.path, oldPath)
     }
 
-    func testInvalidURL() throws {
-          let someCharactersThatAreNotAllowedInPaths = "⊂⧺∀ℝ∀⊂⊤∃∫"
-          for character in someCharactersThatAreNotAllowedInPaths {
-              XCTAssertFalse(CharacterSet.urlPathAllowed.contains(character.unicodeScalars.first!), "Verify that \(character) is invalid")
-
-              let pathWithInvalidCharacter = "/path/with/invalid\(character)for/paths"
-              let source = "@Redirected(from: \(pathWithInvalidCharacter))"
-              let document = Document(parsing: source, options: .parseBlockDirectives)
-              let directive = document.child(at: 0)! as! BlockDirective
-              let (bundle, context) = try testBundleAndContext(named: "TestBundle")
-              var problems = [Problem]()
-              let redirected = Redirect(from: directive, source: nil, for: bundle, in: context, problems: &problems)
-              XCTAssertNil(redirected?.oldPath.absoluteString, "\(character)")
-              XCTAssertFalse(problems.containsErrors)
-              XCTAssertEqual(1, problems.count)
-              XCTAssertEqual(problems.first?.diagnostic.identifier, "org.swift.docc.HasArgument.from.ConversionFailed")
-              XCTAssertEqual(
-                  problems.first?.diagnostic.summary,
-                  "Cannot convert '\(pathWithInvalidCharacter)' to type 'URL'"
-              )
-          }
-      }
-
     func testExtraArguments() throws {
         let oldPath = "/old/path/to/this/page"
         let source = "@Redirected(from: \(oldPath), argument: value)"

diff --git a/Tests/SwiftDocCTests/Utility/ValidatedURLTests.swift b/Tests/SwiftDocCTests/Utility/ValidatedURLTests.swift
@@ -36,19 +36,9 @@ class ValidatedURLTests: XCTestCase {
     }
 
     func testInvalidURLs() {
-        let invalidURLs = [
-            URL(string: "http://:domain")!,
-        ]
-
-        // Test ValidatedURL.init(String)
-        invalidURLs.forEach { url in
-            XCTAssertNil(ValidatedURL(parsingExact: url.absoluteString))
-        }
+        XCTAssertNil(ValidatedURL(parsingExact: "http://:domain"))
 
-        // Test ValidatedURL.init(URL)
-        invalidURLs.forEach { url in
-            XCTAssertNil(ValidatedURL(url))
-        }
+        XCTAssertNil(URL(string: "http://:domain").flatMap { ValidatedURL($0) })
     }
 
     func testRequiringScheme() {

diff --git a/Tests/SwiftDocCUtilitiesTests/ArgumentParsing/ConvertSubcommandSourceRepositoryTests.swift b/Tests/SwiftDocCUtilitiesTests/ArgumentParsing/ConvertSubcommandSourceRepositoryTests.swift
@@ -32,10 +32,10 @@ class ConvertSubcommandSourceRepositoryTests: XCTestCase {
             try assertSourceRepositoryArguments(
                 checkoutPath: "checkout path",
                 sourceService: sourceService,
-                sourceServiceBaseURL: "example.com/path/to/base"
+                sourceServiceBaseURL: "https://example.com/path/to/base"
             ) { action in
                 XCTAssertEqual(action.sourceRepository?.checkoutPath, "checkout path")
-                XCTAssertEqual(action.sourceRepository?.sourceServiceBaseURL, URL(string: "example.com/path/to/base")!)
+                XCTAssertEqual(action.sourceRepository?.sourceServiceBaseURL.absoluteString, "https://example.com/path/to/base")
             }
         }
     }
@@ -55,7 +55,7 @@ class ConvertSubcommandSourceRepositoryTests: XCTestCase {
             try assertSourceRepositoryArguments(
                 checkoutPath: nil,
                 sourceService: nil,
-                sourceServiceBaseURL: "example.com/path/to/base"
+                sourceServiceBaseURL: "https://example.com/path/to/base"
             )
         ) { error in
             XCTAssertEqual(
@@ -124,7 +124,7 @@ class ConvertSubcommandSourceRepositoryTests: XCTestCase {
             try assertSourceRepositoryArguments(
                 checkoutPath: "checkout path",
                 sourceService: "not a supported source service",
-                sourceServiceBaseURL: "example.com/foo"
+                sourceServiceBaseURL: "https://example.com/foo"
             )
         ) { error in
             XCTAssertEqual(