[test] Update alternative interface framework module interface test to run on both arm64 and x86_64 #38464

LucianoPAlmeida · 2021-07-18T00:26:36Z

Running all lit tests on x86 machine cause this test to always fail because it runs a build commands specific for arm64.

Command Output (stderr):
--
<unknown>:0: error: could not find module 'Swift' for target 'arm64-apple-macos'; found: x86_64-apple-macos, at: .../build/Ninja-ReleaseAssert/swift-macosx-x86_64/lib/swift/macosx/Swift.swiftmodule
<unknown>:0: error: could not find module 'Swift' for target 'arm64-apple-macos'; found: x86_64-apple-macos, at: .../build/Ninja-ReleaseAssert/swift-macosx-x86_64/lib/swift/macosx/Swift.swiftmodule

--

********************
********************
Failed Tests (1):
  Swift(macosx-x86_64) :: ModuleInterface/build-alternative-interface-framework.swift


Testing Time: 0.55s
  Failed: 1

This updates the test to use -target %target-cpu-... limited require CPU arch arm64 and x86_64 on macOS and build DummyModule Input module for x86_64 as well.

This was recently reported on the forums https://forums.swift.org/t/build-alternative-interface-framework-failed-while-running-tests/50499

wongzigii · 2021-07-18T15:11:44Z

Thanks!

varungandhi-apple · 2021-07-19T01:33:41Z

See discussion in https://bugs.swift.org/browse/SR-14762, this fix "works" because normally on arm64, you will end up building an arm64 stdlib, but it's not clear if it's the right fix in the first place. If you look at commit in which the test case was added (b6cd513), it seems to be testing the backup directory functionality and it seems like arm64 being hard-coded is not related to thing the test is trying to verify. cc @nkcsgexi

LucianoPAlmeida · 2021-07-19T23:26:45Z

See discussion in https://bugs.swift.org/browse/SR-14762, this fix "works" because normally on arm64, you will end up building an arm64 stdlib, but it's not clear if it's the right fix in the first place. If you look at commit in which the test case was added (b6cd513), it seems to be testing the backup directory functionality and it seems like arm64 being hard-coded is not related to thing the test is trying to verify. cc @nkcsgexi

It seems I've missed that, thanks for the explanation @varungandhi-apple :)
Was also not sure about this, but since everything was hard coded for arm64 it made sense when I looked it up.
I made change on the approach, to run on current target cpu and make sure that the DummyModule is built for x86_64 as well so it can run on both. It seems reasonable to me, but the same as before, not sure is the right fix as well, but let me know if makes sense =]

varungandhi-apple

This seems like a reasonable strategy.

LucianoPAlmeida · 2021-07-20T23:52:55Z

@swift-ci Please test

swift-ci · 2021-07-21T04:11:45Z

Build failed
Swift Test OS X Platform
Git Sha - eba5ede30919d78f50e948f9dc23f9b514c29879

LucianoPAlmeida · 2021-07-21T12:37:44Z

@swift-ci Please test MacOS Platform

LucianoPAlmeida · 2021-07-21T12:47:30Z

It seems that tests run in more targets CPUs than just arm64 and x86, so I'm not sure if we should add all in this case... trying to limit macOS(which I not sure only runs on that because I don't know if even then it targets simulator archs), what do you think @varungandhi-apple?

varungandhi-apple · 2021-07-21T16:50:40Z

Maybe we should go for // REQUIRES: CPU=arm64 || CPU=x86_64 and leave a comment describing why that was added? That at least avoids the one-arch specific issue.

I'm all ears if you have any other ideas...

LucianoPAlmeida · 2021-07-21T21:00:28Z

Maybe we should go for // REQUIRES: CPU=arm64 || CPU=x86_64 and leave a comment describing why that was added? That at least avoids the one-arch specific issue.

I'm all ears if you have any other ideas...

I think it would be fine, different from only // REQUIRES: CPU=arm64 people working on x86_64 Macs would be able to spot regressions locally as well :)
Just a question, // REQUIRES: OS=macosx works as well, so it will run in all currently supported archs for macOS which is only x86_64 and arm64, so question is if you think this is an option too?

varungandhi-apple · 2021-07-21T21:10:36Z

macOS can also run arm64e but that's not supported for 3rd parties right now. arm64e is not substantially different from arm64, so it's okay to leave it out here. I think it's preferable to have checks for both the CPU and the OS in place.

LucianoPAlmeida · 2021-07-21T23:09:17Z

macOS can also run arm64e but that's not supported for 3rd parties right now. arm64e is not substantially different from arm64, so it's okay to leave it out here. I think it's preferable to have checks for both the CPU and the OS in place.

Ah make sense, just made the changes!
Thanks for the reference

LucianoPAlmeida · 2021-07-21T23:17:28Z

@swift-ci Please test

LucianoPAlmeida · 2021-07-22T03:43:27Z

Windows test failure its not related...

LucianoPAlmeida · 2021-07-22T12:11:35Z

@swift-ci Please test Windows Platform

LucianoPAlmeida · 2021-07-22T23:04:57Z

@varungandhi-apple There are any additional points or this is good to go?

…6_64

…mic target cpu

…m64e

LucianoPAlmeida · 2021-07-28T23:17:51Z

Added DummyFramework to build for arm64e as well so we restrict the test only for macOS, let me know if this makes sense :)

varungandhi-apple · 2021-07-29T00:55:14Z

Sorry, I missed your earlier comment. Yeah, I think this is also okay.

@swift-ci test

swift-ci · 2021-07-29T02:44:13Z

Build failed
Swift Test OS X Platform
Git Sha - 1fc7f07

LucianoPAlmeida · 2021-07-29T02:47:21Z

Sorry, I missed your earlier comment. Yeah, I think this is also okay.

No worries @varungandhi-apple, thanks for the review :)

LucianoPAlmeida · 2021-07-29T11:49:48Z

@swift-ci Please test MacOS Platform

LucianoPAlmeida requested a review from varungandhi-apple July 18, 2021 00:31

LucianoPAlmeida force-pushed the require-arch-for-test branch from 41af21f to eba5ede Compare July 19, 2021 23:21

varungandhi-apple approved these changes Jul 20, 2021

View reviewed changes

LucianoPAlmeida force-pushed the require-arch-for-test branch from eba5ede to 90c6eff Compare July 21, 2021 12:37

LucianoPAlmeida requested a review from varungandhi-apple July 21, 2021 12:37

LucianoPAlmeida force-pushed the require-arch-for-test branch from 90c6eff to e3d7b5d Compare July 21, 2021 23:04

LucianoPAlmeida changed the title ~~[test] Require arm64 for test target that run target specific commands~~ [test] Update alternative interface framework module interface test to run on both arm64 and x86_64 Jul 22, 2021

LucianoPAlmeida force-pushed the require-arch-for-test branch from e3d7b5d to 3237806 Compare July 24, 2021 17:21

LucianoPAlmeida added 3 commits July 28, 2021 18:35

[tests] Make sure to build module interface test dummy library for x8…

eb70e0f

…6_64

[tests] Run module interface build alternative interface test in dyna…

c5ad287

…mic target cpu

[tests] Make sure to build module interface test dummy library for ar…

1fc7f07

…m64e

LucianoPAlmeida force-pushed the require-arch-for-test branch from 3237806 to 1fc7f07 Compare July 28, 2021 23:14

LucianoPAlmeida requested a review from nkcsgexi July 28, 2021 23:18

varungandhi-apple approved these changes Jul 29, 2021

View reviewed changes

LucianoPAlmeida mentioned this pull request Jul 29, 2021

C++ Interop: import const methods as non-mutating #38618

Merged

LucianoPAlmeida merged commit 022d44d into swiftlang:main Jul 29, 2021

LucianoPAlmeida deleted the require-arch-for-test branch July 29, 2021 21:44

LucianoPAlmeida mentioned this pull request Jul 29, 2021

[SR-14762] Fix test compiling for arm64 macOS without stdlib check #57112

Closed

[test] Update alternative interface framework module interface test to run on both arm64 and x86_64 #38464

[test] Update alternative interface framework module interface test to run on both arm64 and x86_64 #38464

Uh oh!

Conversation

LucianoPAlmeida commented Jul 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wongzigii commented Jul 18, 2021

Uh oh!

varungandhi-apple commented Jul 19, 2021

Uh oh!

LucianoPAlmeida commented Jul 19, 2021

Uh oh!

varungandhi-apple left a comment

Choose a reason for hiding this comment

Uh oh!

LucianoPAlmeida commented Jul 20, 2021

Uh oh!

swift-ci commented Jul 21, 2021

Uh oh!

LucianoPAlmeida commented Jul 21, 2021

Uh oh!

LucianoPAlmeida commented Jul 21, 2021

Uh oh!

varungandhi-apple commented Jul 21, 2021

Uh oh!

LucianoPAlmeida commented Jul 21, 2021

Uh oh!

varungandhi-apple commented Jul 21, 2021

Uh oh!

LucianoPAlmeida commented Jul 21, 2021

Uh oh!

LucianoPAlmeida commented Jul 21, 2021

Uh oh!

LucianoPAlmeida commented Jul 22, 2021

Uh oh!

LucianoPAlmeida commented Jul 22, 2021

Uh oh!

LucianoPAlmeida commented Jul 22, 2021

Uh oh!

LucianoPAlmeida commented Jul 28, 2021

Uh oh!

varungandhi-apple commented Jul 29, 2021

Uh oh!

swift-ci commented Jul 29, 2021

Uh oh!

LucianoPAlmeida commented Jul 29, 2021

Uh oh!

LucianoPAlmeida commented Jul 29, 2021

Uh oh!

Uh oh!

LucianoPAlmeida commented Jul 18, 2021 •

edited

Loading