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

Add support for running iOS unit tests on Bitrise #3379

Merged
merged 2 commits into from
May 29, 2020

Conversation

jacob-carlborg
Copy link
Contributor

This setups up a CI pipeline for iOS to run on http://bitrise.io. It consists of a template for an Xcode project and a bitrise.yml file to configure the pipeline. When running ldc-build-runtime, the template will be used to create a real Xcode project. The Xcode project will run the unit tests for both druntime and Phobos.

This will run the unit tests on a real physical device on Bitrise.

For this review, the main interesting files would be:

  • bitrise.yml
  • runtime/TestRunnerTemplate/TestRunnerTests/TestRunnerTests.m
  • runtime/ldc-build-runtime.d.in

For this to work, an account on Bitrise needs to be connected to this repository, an Apple developer account needs to be connected to the Bitrise account and the Bitrise workflow needs to be configured to use the bitrise.yml file.

Example of test run: https://app.bitrise.io/build/b6bb5ec7bbb732f2.

@kinke
Copy link
Member

kinke commented Mar 22, 2020

Looking pretty good (bitrise run, haven't looked at the changes yet). It should currently error out though:

****** FAIL debug64 std.socket
std.socket.SocketParameterException@std/socket.d(2007): Path too long

@jacob-carlborg
Copy link
Contributor Author

jacob-carlborg commented Mar 22, 2020

It should currently error out though:

Yes, I noticed. It worked before, not sure what's changed (might not have been a large enough margin for the path length). It's a simple fix though. The path of a Unix socket file is too long, just need to reduce length of the path. Unfortunately that means that not all of the unit tests pass in the new LDC beta.

@kinke
Copy link
Member

kinke commented Mar 22, 2020

It's a simple fix though.

Sure, but what I meant was that the CI run should fail if an error occurs. So it's good that there's a failure. ;)

@jacob-carlborg
Copy link
Contributor Author

Sure, but what I meant was that the CI run should fail if an error occurs. So it's good that there's a failure.

I though it was a "soft" unit test (some of the unit tests in std.socket are allowed to fail). But I see now that it's not a soft unit test. I guess I have some more work to do 😄.

bitrise.yml Outdated Show resolved Hide resolved
bitrise.yml Show resolved Hide resolved
@kinke
Copy link
Member

kinke commented Mar 23, 2020

For this to work, an account on Bitrise needs to be connected to this repository, an Apple developer account needs to be connected to the Bitrise account and the Bitrise workflow needs to be configured to use the bitrise.yml file.

Would it be possible to simply connect your existing bitrise account to this repo?

@jacob-carlborg
Copy link
Contributor Author

jacob-carlborg commented Mar 23, 2020

Would it be possible to simply connect your existing bitrise account to this repo?

I think so.

Yes, but I don't have permission to setup a webhook. So the builds won't be triggered automatically. Either an existing LDC member has to do it or if you would like to add me as a member of ldc-developers then I can do it.

@jacob-carlborg
Copy link
Contributor Author

Sure, but what I meant was that the CI run should fail if an error occurs. So it's good that there's a failure. ;)

The CI pipeline fails now when the unit tests fail [1]. It also now prints the logs when the unit tests fail, so you can actually see what failed.

[1] https://app.bitrise.io/build/146accf172ac513d

@jacob-carlborg
Copy link
Contributor Author

I think I've addressed all comments now.

The fix for the unit test is available here: dlang/phobos#7430.

@kinke
Copy link
Member

kinke commented Mar 25, 2020

Yes, but I don't have permission to setup a webhook. So the builds won't be triggered automatically. Either an existing LDC member has to do it or if you would like to add me as a member of ldc-developers then I can do it.

I can do that (I think), just need URL, content type (JSON or x-www-form-urlencoded), 'secret' and triggering events (I guess Pull requests and Pushes). You can send me the sensitive stuff via gitter (assuming that's halfway safe).

@kinke
Copy link
Member

kinke commented Mar 26, 2020

Great, that's working now. There's apparently a new regression though, possibly related to the changed bitrise config:

error: No signing certificate "iOS Development" found: No "iOS Development" signing certificate matching team ID "DGM3Z2TJ7K" with a private key was found.

@jacob-carlborg
Copy link
Contributor Author

error: No signing certificate "iOS Development" found: No "iOS Development" signing certificate matching team ID "DGM3Z2TJ7K" with a private key was found.

I'm aware. It's something that needs to be setup for each project. It's not related to the bitrise.yml file. It needs to be done in the Bitrise UI, which I've done. Not sure why it doesn't work. It has worked on two other projects.

@jacob-carlborg
Copy link
Contributor Author

Rebased to latest master.

@kinke
Copy link
Member

kinke commented May 21, 2020

Thx. - The apparent segfault regression while compiling the core.lifetime unittests is interesting:

4  libsystem_platform.dylib 0x0000000000000002 _sigtramp + 18446603338526799042
5  ldc2                     0x0000000106dff40e llvm::Value::setName(llvm::Twine const&) + 14
6  ldc2                     0x0000000104f1669d DtoDefineFunction(FuncDeclaration*, bool) + 4429
7  ldc2                     0x0000000104f00bad CodegenVisitor::visit(TemplateInstance*) + 333

@kinke
Copy link
Member

kinke commented May 21, 2020

Better call stack from my build:

Assertion failed: t == IrDsymbol::ParamterType || t == IrDsymbol::NotSet, file C:\LDC\ldc\ir\irvar.cpp, line 87
 #5 0x00007ff63a7dc2c1 isIrParameterCreated(class VarDeclaration *) C:\LDC\ldc\ir\irvar.cpp:87:0
 #6 0x00007ff63a7dc200 getIrParameter(class VarDeclaration *, bool) C:\LDC\ldc\ir\irvar.cpp:77:0
 #7 0x00007ff63a7eca60 `anonymous namespace'::defineParameters C:\LDC\ldc\gen\functions.cpp:818:0
 #8 0x00007ff63a7eb8ac DtoDefineFunction(class FuncDeclaration *, bool) C:\LDC\ldc\gen\functions.cpp:1246:0

The type seems to be LocalType. Doesn't happen for Linux AArch64 etc.

@kinke
Copy link
Member

kinke commented May 21, 2020

Ah, might be related to the ABI fixes, where empty-struct by-value arguments aren't passed anymore for iOS targets.

@jacob-carlborg
Copy link
Contributor Author

The ABI documentation says:

In the generic ABI, empty structs are treated as aggregates with a single byte member for parameter passing. In iOS, however, they are ignored unless they have a nontrivial destructor or copy-constructor. If they do have such functions, they are considered as aggregates with one byte member in the generic manner.

https://developer.apple.com/library/archive/documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARM64FunctionCallingConventions.html#//apple_ref/doc/uid/TP40013702-SW1

@jacob-carlborg
Copy link
Contributor Author

The build is green now on Bitrise [1] since I've now excluded running the unit tests for PRs.

[1] https://app.bitrise.io/build/ecb6a13c4e43ebcb

@kinke
Copy link
Member

kinke commented May 22, 2020

Thx. I take it that's a bitrise UI setting, as the yaml seems untouched.

I should probably transplant the unrelated ICE fix to a dedicated PR (and add a testcase).

@jacob-carlborg
Copy link
Contributor Author

Thx. I take it that's a bitrise UI setting, as the yaml seems untouched.

No, it's in the YAML file: https://github.com/jacob-carlborg/ldc/blob/64bb0dd5ecbb08a9cada501c2ab9945e7dd7815b/bitrise.yml#L86

I should probably transplant the unrelated ICE fix to a dedicated PR (and add a testcase).

👍.

This will run the unit tests for druntime and Phobos on a physical
device (iPhone).
@kinke
Copy link
Member

kinke commented May 29, 2020

Alright, let's find out whether the unittests are run successfully for the master branch. - Thx Jacob!

@kinke kinke merged commit cf0c9e6 into ldc-developers:master May 29, 2020
@kinke
Copy link
Member

kinke commented May 29, 2020

Argh, they've just removed the iPhone6 + iOS 12.2 combo 12 hours ago: bitrise-steplib/steps-virtual-device-testing-for-ios@801c27b#diff-91d8d269a1c9bd3631ddb04b80b01c4e

Please choose another config of your liking.

@jacob-carlborg jacob-carlborg deleted the ios-testing branch May 30, 2020 06:56
@jacob-carlborg
Copy link
Contributor Author

Please choose another config of your liking.

#3450

@jacob-carlborg
Copy link
Contributor Author

Strange that they removed the one iOS version that was not deprecated and left the one that was. So much for deprecation 😞.

@kinke
Copy link
Member

kinke commented Jul 19, 2020

@jacob-carlborg: It'd be great if you could look into the apparent std.getopt regression. All the logs show is a stack trace, without printing a failing assertion.

@jacob-carlborg
Copy link
Contributor Author

It'd be great if you could look into the apparent std.getopt regression. All the logs show is a stack trace, without printing a failing assertion.

Sure, I'll try to do that.

@jacob-carlborg
Copy link
Contributor Author

Looking at the stacktrace, it's possible that there is no assertion failure but a crash, which the XCTest framework catches. I'm hoping symbols will be demangled if it's a D assertion that is triggered. In this stacktrace the D symbols are not demangled.

@jacob-carlborg
Copy link
Contributor Author

jacob-carlborg commented Jul 19, 2020

I've done some more investigation. If LDC is linked against LLVM 9, everything works fine. If LDC is linked against LLVM 10, it crashes in the unit test runner. It crashes here [1], with an EXC_BAD_ACCESS, when calling the unit test function.

[1] https://github.com/ldc-developers/druntime/blob/19731a92a97dbe4d7f7a4e15ceaff8444a1f879a/src/test_runner.d#L73

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.

2 participants