Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Driver] Begin testing -driver-show-incremental #5128

Merged

Conversation

modocache
Copy link
Contributor

SR-2855 describes additional functionality to be added to -driver-show-incremental. That option is not currently tested anywhere. Using the option does not interfere with existing incremental compilation tests, so begin adding it to tests, starting with mutual dependencies tests.

I considered adding new, isolated tests for this option, but adding it to existing tests results in less setup code and test boilerplate.

A first step in addressing SR-2855.


@jrose-apple I thought adding some tests for -driver-show-incremental would be a good start. What do you think of adding it to the existing tests? If that sounds good, I'll add it to more tests, then update the CHECKs once I've implemented SR-2855.


// CHECK-FIRST-NOT: warning
// CHECK-FIRST: Handled main.swift
// CHECK-FIRST: Handled other.swift
// CHECK-FIRST: Queuing main.swift (initial)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. I'm a little concerned about merging the two output streams this way. This seems like odd behavior, seeing the "Queuing" line after the things are actually handled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I've moved the test to its own file. My plan is to, in future commits, add files like test/Driver/Dependencies/driver-show-incremental-*.swift for each interesting output in -driver-show-incremental. Does that sound good?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that should be sufficient. I like the idea of it just coming with any test added to Driver/Dependencies/, but in practice it seems like more trouble than it's worth.

https://bugs.swift.org/browse/SR-2855 describes additional
functionality to be added to `-driver-show-incremental`. That option is
not currently tested anywhere. Begin adding tests that mirror
incremental compilation tests, like `test/Driver/Dependencies/mutual.swift`.
@modocache modocache force-pushed the sr-2855-driver-show-incremental-tests branch from abe6158 to f2953de Compare October 8, 2016 21:19
@modocache
Copy link
Contributor Author

modocache commented Oct 10, 2016

Cool, thanks! I'll go ahead and merge this, then:

  1. In a future pull request this week, I'll add many more of this sort of test.
  2. Then, I'll make changes that address SR-2855, adding additional CHECK verifications to the tests as I go.

@modocache
Copy link
Contributor Author

@swift-ci Please test and merge

@jrose-apple
Copy link
Contributor

I don't actually expect SR-2855 to intersect much with other tests; those are the cases where nothing incremental is happening!

@modocache
Copy link
Contributor Author

Ah, hmm, good point. Then I guess this is sort of tangential -- although it is helping me understand -driver-show-incremental, so I'll do it anyway. :)

The build failures are due to an unrelated error in SwiftPM. I'll try to merge again later.

@modocache
Copy link
Contributor Author

@swift-ci please test and merge

@modocache
Copy link
Contributor Author

@swift-ci please clean test and merge

@swift-ci swift-ci merged commit ba0dc90 into swiftlang:master Oct 11, 2016
@modocache modocache deleted the sr-2855-driver-show-incremental-tests branch October 11, 2016 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants