Skip to content

Loopback tests for URLSession #613

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

Merged
merged 1 commit into from
Sep 28, 2016
Merged

Conversation

pushkarnk
Copy link
Member

@pushkarnk pushkarnk commented Aug 31, 2016

This is a very basic HTTP server for loopback tests for URLSession. I have borrowed some implementation aspects from the HTTPServer.swift in PR 299. The server currently, listens, reads and writes only once. It is possible to test data tasks and download tasks. Opening this PR for review and comments.

I also had to make a small change to build.py to get TestFoundation built with Dispatch.

] + glob.glob('./TestFoundation/Test*.swift')) # all TestSomething.swift are considered sources to the test project in the TestFoundation directory

Configuration.current.extra_ld_flags = '-L'+Configuration.current.variables["LIBDISPATCH_BUILD_DIR"]+'/src/.libs'
Copy link
Member Author

@pushkarnk pushkarnk Aug 31, 2016

Choose a reason for hiding this comment

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

@seabaylea : In build.py, TestFoundation is built as a SwiftExecutable without the linker options, this results in /usr/bin/ld.gold failing to locate libdispatch. I can see that a the rule for SwiftExecutable uses EXTRA_LD_FLAGS. Hence added this line. Can you please comment on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense - its what we have to do when building Foundation as Dispatch isn't yet in the right place (as we're still building the toolchain)

@pushkarnk pushkarnk force-pushed the loopback branch 2 times, most recently from 51a79b4 to 3bd7f80 Compare September 7, 2016 14:46
@pushkarnk
Copy link
Member Author

pushkarnk commented Sep 7, 2016

Added support to test download tasks. I need to work on making this build and run on Darwin next.

@parkera
Copy link
Contributor

parkera commented Sep 12, 2016

How's this going?

@pushkarnk
Copy link
Member Author

pushkarnk commented Sep 14, 2016

I need to make this build and run on Darwin.

@pushkarnk pushkarnk force-pushed the loopback branch 3 times, most recently from 5df83f7 to 410bfbe Compare September 15, 2016 15:06
@pushkarnk
Copy link
Member Author

@parkera : I've updated the Xcode project. Can you please run a test?

typealias FoundationObject = NSObject
#else
typealias FoundationObject = SwiftFoundation.NSObject
#endif
Copy link
Member Author

Choose a reason for hiding this comment

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

This looks awkward. But I found the use of NSObject causing a "NSObject ambiguous for type lookup in this context" compiler error with Xcode.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to understand more about the root cause of this error. Clients of Foundation should absolutely be able to subclass NSObject without this kind of hackery.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Removed this.

@pushkarnk
Copy link
Member Author

The ambiguity error comes from the import Dispatch statement in TestFoundation/TestNSURLSession. I have created https://bugs.swift.org/browse/SR-2752 to investigate this. Looks like Dispatch also exposes NSObject. I removed the dirty hack and did some refactoring to work around the reported problem.

@@ -467,8 +467,11 @@
# TODO: Probably this should be another 'product', but for now it's simply a phase
foundation_tests = SwiftExecutable('TestFoundation', [
'TestFoundation/main.swift',
'TestFoundation/HTTPServer.swift',
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this file go in the swift_sources list?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure. But swift_sources seems to be holding all the swift source files that are compiled into Foundation. HTTPServer.swift needs to be compiled into TestFoundation.

Copy link
Contributor

@parkera parkera Sep 26, 2016

Choose a reason for hiding this comment

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

You're right, the other test files are found via the glob. I guess this is ok. I'm going to kick off a test.

@parkera
Copy link
Contributor

parkera commented Sep 26, 2016

@swift-ci please test

@parkera parkera merged commit cc5985e into swiftlang:master Sep 28, 2016
@pushkarnk pushkarnk deleted the loopback branch September 28, 2016 18:19
pushkarnk pushed a commit to pushkarnk/swift-corelibs-foundation that referenced this pull request Sep 29, 2016
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