Some Improvements to Cross-Module Incremental Build Testing #541
Conversation
* Document paths * Use build directory terminology that is consistent with XCBuild * Fixup the executable path computation for Windows
writeFileContents can fail if the file already exists.
|
@swift-ci test |
There was a problem hiding this comment.
Very nice tests and improvements to the framework! The only change I can think of to suggest: When defining a module in a test that may not import some other module, define a different module object (with the same name) that does not claim to import the other module.
Example:
let M1 = Module(named: "M", containing: "X", importing: [], ...)
let M2 = Module(named: "M", containing: "X", importing: [N], ...)
Then use M1 in a step in which there is no import line (or it's commented-out) in X, and use M2 when there is.
| @@ -0,0 +1,107 @@ | |||
| //===---------------- Antisymmetry.swift - Swift Testing ------------------===// | |||
There was a problem hiding this comment.
This whole file is beautiful! Well-done!! I wonder if, in the future, we'll want to add other sorts of uses and definitions, and/or test removals as well?
| static var A = Module(named: "A", containing: [ | ||
| .A, | ||
| ], importing: [ | ||
| .B, .C, |
There was a problem hiding this comment.
As I read this--haven't absorbed the whole thing yet--I'm wondering why A also imports C? Or would that be a different test we might need?
There was a problem hiding this comment.
Hmmm... if the import is optional, maybe there should be different Module A objects, one with- and another without the import.
There was a problem hiding this comment.
All imports does is configure search paths, right? I'm not worried about having to state this conditional dependency edge all the time.
There was a problem hiding this comment.
It's a slightly tighter test without the imports because then it relies a bit less on our assumptions.
There was a problem hiding this comment.
It'll also be noisier because the change in search paths will force us to discard the incremental build state. We need separate tests for that case if we don't have them already.
There was a problem hiding this comment.
Excellent point! Thank you for thinking of it.
| .expecting(.init(always: [ .A ]))), | ||
| // Finally, add a member to a struct in C, which influences the layout of | ||
| // the struct in B, which influences the layout of the struct in A. | ||
| // Everything rebuilds! |
There was a problem hiding this comment.
Great to see this test!
|
⛵ |
Add some more tests, and touch up some of the internals of the test framework.