-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Enable SourceKit tests if building SourceKit #8485
Conversation
@swift-ci please smoke test |
It seems that marker affect to location in response on macOS. |
@swift-ci please smoke test |
Sorry, it also needs to fix expecting response. 🙇 |
@swift-ci please smoke test |
Why are so many of the tests XFAILd on Linux though? If they actually require objc_interop, you should add 'REQUIRES: objc_interop' instead. |
@slavapestov I don't yet investigate each failing tests, but we have failing tests in SourceKitten on Swift 3.1 for Linux too. jpsim/SourceKitten#354 |
Those tests in SourceKitten have passed on Swift 3.0.2 for Linux. |
…td object did not resolve to a known type" on Linux
e11ea91
to
0c2a446
Compare
@slavapestov Hi, I classified test results and split them into commits:
|
Write two crash log patterns here:
|
filed: |
@swift-ci please smoke test |
I want to help fixing those. Also, if anyone wants to be my mentor, please do be :) Edit🤔 nevermind, they are not tagged as such. I think I'll go for some starters beforehand, then: https://t.co/qDTn47DBVY |
686e198
to
4b33e15
Compare
Looks like this merge is to indicate for the failures in: https://ci.swift.org/job/oss-swift-incremental-RA-linux-ubuntu-14_04/2230/ I started a revert PR. |
Does that failure occur with only ubuntu 14.04? 🤔 |
It would seem so I have not seen a failure on the 16_04 bot: https://ci.swift.org/job/oss-swift-incremental-RA-linux-ubuntu-16_04/ |
This appeared on the stderr: terminate called after throwing an instance of 'std::bad_alloc'
what(): std::bad_alloc
pure virtual method called
terminate called recursively Could it be a call bug that only occurs in 14.04? |
I can not reproduce the test failure on local ubuntu 14.04. 😕 |
The test that failed was ******************** TEST 'Swift(linux-x86_64) :: SourceKit/ExtractComment/extract_comments.swift' FAILED ******************** Not sure if helps. |
It seems that the revert did fix the bot: We have had several builds since: https://ci.swift.org/job/oss-swift-incremental-RA-linux-ubuntu-14_04/2242 that were blue. |
Right so now the issue is to find out why the build failed on 14.04 so that we can fix it and make it run again, or to disable the test from running (or from running on 14.04 specifically). The point is these tests have never run on Linux by default so it's not impossible that we're actually finding a real issue with the code on 14.04. |
Looks non-deterministic 2230: SourceKit/ExtractComment/extract_comments.swift' FAILED
2231: SourceKit/CodeFormat/indent-switch.swift' FAILED
2232: PASS 2233: SourceKit/CodeFormat/indent-switch.swift' FAILED
2234: SourceKit/CodeFormat/indent-switch.swift' FAILED
2235: SourceKit/CodeFormat/indent-with-tab.swift' FAILED
2236: SourceKit/CodeFormat/indent-closure.swift' FAILED
2237: PASS 2239: SourceKit/CodeFormat/indent-computed-property.swift' FAILED
2240: SourceKit/CodeFormat/indent-bracestmt2.swift' FAILED
2241: SourceKit/CodeFormat/indent-basic.swift' FAILED
|
Yes, it looks like we are running out of memory. I cannot judge whether this is an existing error (that we happen to observe because of system characteristics of that bot) or due to some of the source code changes in this commit. Maybe the machine does not have enough memory? I am not familiar with SourceKit. @akyrtzi Are you fine with just disabling the failing tests on linux (only 14_04) and taking this change? I would say yes since these tests did not run before on linux. |
I have run tests on low memory 14.04 vm, got all tests of SourceKit have passed and another tests crashed. |
I'm fine with disabling for 14_04. |
Is there a best practice way of disabling these tests for the 14.04 build only? |
You could add an available_feature that references the OS version. See test/lit.config. We don't have a available_feature for identifying the linux version as far as i can tell by quickly glancing over this file. |
How about this?: diff --git a/test/SourceKit/lit.local.cfg b/test/SourceKit/lit.local.cfg
index e9e550f518..42dad58ea9 100644
--- a/test/SourceKit/lit.local.cfg
+++ b/test/SourceKit/lit.local.cfg
@@ -1,6 +1,9 @@
if 'sourcekit' not in config.available_features:
config.unsupported = True
+elif 'OS=linux-gnu' in config.available_features and 'Linux=Ubuntu-14.04' in config.available_features:
+ config.unsupported = True
+
else:
config.sourcekitd_test = config.inferSwiftBinary('sourcekitd-test')
config.complete_test = config.inferSwiftBinary('complete-test')
diff --git a/test/lit.cfg b/test/lit.cfg
index af64c5d665..bf7c691993 100644
--- a/test/lit.cfg
+++ b/test/lit.cfg
@@ -1085,4 +1085,24 @@ if config.lldb_build_root != "":
python_lib_dir = get_python_lib(True, False, config.lldb_build_root)
config.substitutions.append(('%lldb-python-path', python_lib_dir))
+
+# Run lsb_release on the target to be tested and return the results.
+def linux_get_lsb_release(commandPrefix=[]):
+ distributor = lit.util.executeCommand(
+ commandPrefix + ['/usr/bin/lsb_release', '-is'])[0].rstrip()
+ release = lit.util.executeCommand(
+ commandPrefix + ['/usr/bin/lsb_release', '-rs'])[0].rstrip()
+ codename = lit.util.executeCommand(
+ commandPrefix + ['/usr/bin/lsb_release', '-cs'])[0].rstrip()
+ return (distributor, release, codename)
+
+if platform.system() == 'Linux':
+ (distributor, release, codename) = linux_get_lsb_release()
+ if (distributor == '' or release == '' or codename == ''):
+ lit_config.fatal('Could not get or decode lsb_release output.')
+ config.available_features.add("Linux=" + distributor + '-' + release)
+ lit_config.note(
+ 'Running tests on %s version %s (%s)' %
+ (distributor, release, codename))
+
lit_config.note("Available features: " + ", ".join(config.available_features)) Updated: move printing "Available features: " to tail |
Instead of fatally aborting the lit_config, I would simply not set the Linux=Ubuntu-14.04 if you can't find the values. You might also want to use LinuxDistribution or LinuxLSBRelease instead, in case a future version uses Linux for other purposes. Otherwise I think this approach will work well. |
Thanks for feedback. |
sourcekit-inproc
fromsourcekitdInProc.framework
tolibsourcekitdInProc.so
on non Darwin platformsResolves SR-1676.