Improve test coverage calculation #102
Conversation
dfed
force-pushed
the
dfed--test-coverage
branch
2 times, most recently
from
ed6c7e6 to
1b26c48
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #102 +/- ##
==========================================
- Coverage 99.67% 99.66% -0.01%
==========================================
Files 47 33 -14
Lines 11564 2678 -8886
==========================================
- Hits 11526 2669 -8857
+ Misses 38 9 -29
|
dfed
force-pushed
the
dfed--test-coverage
branch
6 times, most recently
from
a9446d6 to
2d00400
Compare
| @@ -23,14 +23,11 @@ import SwiftSyntax | |||
| extension ImportDeclSyntax { | |||
| // MARK: Public | |||
|
|
|||
| public var asImportStatement: ImportStatement? { | |||
| guard let moduleName = path.first?.name.text else { | |||
There was a problem hiding this comment.
we really don't need to guard here – if our code-gen is running then our build succeeded, which means we don't have any dangling import statements.
| @@ -41,38 +41,32 @@ public enum FixableInjectableError: DiagnosticError { | |||
| // MARK: - InjectableDiagnosticMessage | |||
|
|
|||
| private struct InjectableDiagnosticMessage: DiagnosticMessage { | |||
| var diagnosticID: MessageID { | |||
There was a problem hiding this comment.
these computed properties were never executed in tests. There's no good way to do so as part of an integration test. So now we're just creating them up-front.
| @@ -105,7 +105,9 @@ public final class DependencyTreeGenerator { | |||
| .map(\.asSource) | |||
| .joined(separator: " -> ")) | |||
| """ | |||
| }.joined(separator: "\n")) | |||
| } | |||
| .sorted() | |||
There was a problem hiding this comment.
we didn't have a test for multiple fulfillable properties! once I added one, it became clear that the error wasn't stable, so we now sort here.
| @@ -136,11 +138,7 @@ public final class DependencyTreeGenerator { | |||
| } | |||
| } | |||
|
|
|||
| struct UnfulfillableProperty: Hashable, Comparable { | |||
There was a problem hiding this comment.
we don't need the property to be comparable because the generated error is already comparable and we sort that
| @@ -153,32 +151,25 @@ public final class DependencyTreeGenerator { | |||
| private let typeDescriptionToFulfillingInstantiableMap: [TypeDescription: Instantiable] | |||
| private var rootScopeGenerators: [ScopeGenerator] { | |||
| get throws { | |||
| if let _rootScopeGenerators { | |||
There was a problem hiding this comment.
we never actually used the cached object. so now it's gone.
| // We can't find an instantiable for this type. | ||
| // This is bad, but we handle this error in `validateReachableTypeDescriptions()`. | ||
| return |
There was a problem hiding this comment.
this was impossible to hit, and we already have validateReachableTypeDescriptions() to handle less egregious cases. So I folded in this guard let with the one below
| @@ -222,28 +222,6 @@ public enum TypeDescription: Codable, Hashable, Comparable, Sendable { | |||
| } | |||
| } | |||
|
|
|||
| var isUnknown: Bool { | |||
There was a problem hiding this comment.
this was only used in tests – so I moved it to tests
| @@ -423,8 +401,6 @@ extension ExprSyntax { | |||
| if memberAccessExpr.declName.baseName.text == "self" { | |||
| if let base = memberAccessExpr.base { | |||
| return base.typeDescription | |||
| } else { | |||
| return .unknown(text: memberAccessExpr.trimmedDescription) | |||
| @@ -440,8 +416,6 @@ extension ExprSyntax { | |||
| generics: [] | |||
| ) | |||
| } | |||
| } else { | |||
| return .unknown(text: memberAccessExpr.trimmedDescription) | |||
| @@ -508,8 +482,6 @@ extension ExprSyntax { | |||
| doesThrow: arrow.effectSpecifiers?.throwsSpecifier != nil, | |||
| returnType: returnType.typeDescription | |||
| ) | |||
| } else { | |||
| return .unknown(text: trimmedDescription) | |||
| } | ||
| return .unknown(text: trimmedDescription) |
There was a problem hiding this comment.
I hate fall through like this but... it does work
| @@ -2685,7 +2685,7 @@ final class SafeDIToolCodeGenerationTests: XCTestCase { | |||
| } | |||
| """, | |||
| """ | |||
| import Foundation | |||
| @_implementationOnly import Foundation | |||
There was a problem hiding this comment.
helps exercise some of our import handling code we previously were not
So our test coverage has been cheating a bit – we were including our
Tests/code in our coverage numbers. Now we aren't. To get us back to 99.5+% coverage I added a few more tests. I also mucked with the shape of some lines to make it easier to get coverage – I'm not proud of it... but... here we are.