Skip to content

Conversation

@artemcm
Copy link
Contributor

@artemcm artemcm commented Apr 15, 2021

#599 and #602 disabled the following tests:

  • testDarwinBasic
  • testSaveTemps
  • testInputForwarding
  • testPrebuiltModuleGenerationJobs
    This PR generalizes those tests to be agnostic to the macOS architecture and re-enables them on Apple Silicon.

The only remaining Apple Silicon-disabled test after this is:

  • testLitDriverDependenciesTests

@artemcm artemcm requested a review from nkcsgexi April 15, 2021 21:32
job.moduleName == "MissingKit"
})
XCTAssertTrue(jobs.count == 13)
XCTAssertTrue(jobs.count == 18)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nkcsgexi , I may not have the full context for some of the things being tested here so please take a look to ensure it's still testing the right thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for fixing this! The test updates look correct to me.

job.moduleName == "MissingKit"
})
XCTAssertTrue(jobs.count == 13)
XCTAssertTrue(jobs.count == 18)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for fixing this! The test updates look correct to me.

$0 <<< "import H\n"
$0 <<< "import Swift\n"
}
var driver = try Driver(args: ["swiftc", main.pathString,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to always use the host triple? Can we give a specific -target here to make the test arch-agnostic?

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'm having a hard time reproducing the original CI failures for this test so I'm investigating why that may be.

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 finally reproduced this locally, and the failure is because these are some of the few tests actually need to link to succeed (as opposed to just invoking the driver and inspecting the generated jobs). And when being tested with a just-built compiler, we do not have a fully-formed fat toolchain, we just have the host slice of everything.

So I think in this case it's simpler to just compile for the host to make sure these tests are still exercised on whatever platform we are testing on.

@artemcm artemcm force-pushed the AppleSiliconTestRestore branch from d587ab4 to fd336b6 Compare April 21, 2021 17:16
@artemcm
Copy link
Contributor Author

artemcm commented Apr 27, 2021

@swift-ci please test

@artemcm artemcm merged commit 9e2c870 into swiftlang:main Apr 28, 2021
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