Skip to content

Mock out test interactions with swift.org and github.com #133

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 29 commits into from
Jul 22, 2024

Conversation

cmcgee1024
Copy link
Member

@cmcgee1024 cmcgee1024 commented Jul 13, 2024

The unit tests currently perform numerous requests against these websites, which causes problems. First, GitHub API's are rate limited, which is often encountered while running the tests both in CI and at desk since not all environments are prepared to manage their own app/dev token. Downloads from swift.org, usually toolchains, are large, take a significant time for each to download in aggregate during testing. Ultimately, network interactions can fail due to network conditions outside of the control of the tests.

There is already the infrastructure in the tests to mock the downloading of toolchains by locally constructing them with executable shell scripts in place of the real tools. In the case of macOS there is also logic to assemble the .pkg file. Use this mock for the majority of the unit tests.

Create a small set of integration and e2e tests that do make real network requests.

Create static files that represent a mocked snapshot of the GitHub API's response JSON and use those when using the mock toolchain downloader during the automated tests. Add these as resources to the test target so that they can be loaded at the runtime of the tests.

In order to decrease the exposure to network issues related to the Ubuntu keyserver in the automated tests add a latch so that the GPG keys are only refreshed once per session of the swiftly executable. Since swiftly operates normally as a short-lived process this should not affect the real-world usage of it, but it makes a big improvement in both the speed and reliability of the automated tests. It's very unlikely that in the span of a test run that the swift keys will need to be refreshed.

The unit tests currently perform numerous requests against these
websites, which causes problems. First, GitHub API's are rate
limited, which is often encountered while running the tests both
in CI and at desk since not all environments are prepared to manage
their own app/dev token. Downloads from swift.org, usually
toolchains, are large, take a significant time for each to download
in aggregate during testing. Ultimately, network interactions can
fail due to network conditions outside of the control of the tests.

There is already the infrastructure in the tests to mock the
downloading of toolchains by locally constructing them with
executable shell scripts in place of the real tools. In the case of
macOS there is also logic to assemble the .pkg file. Use this mock
for the majority of the unit tests.

TODO: create a small set of integration and e2e tests that do make
real network requests.

Create static files that represent a mocked snapshot of the GitHub
API's response JSON and use those when using the mock toolchain
downloader during the automated tests. Add these as resources to
the test target so that they can be loaded at the runtime of the
tests.

In order to decrease the exposure to network issues related to the
Ubuntu keyserver in the automated tests add a latch so that the GPG
keys are only refreshed once per session of the swiftly executable.
Since swiftly operates normally as a short-lived process this should
not affect the real-world usage of it, but it makes a big improvement
in both the speed and reliability of the automated tests. It's very
unlikely that in the span of a test run that the swift keys will
need to be refreshed.
@cmcgee1024
Copy link
Member Author

@swift-ci test macOS

@cmcgee1024
Copy link
Member Author

@swift-server-bot test this please

@cmcgee1024
Copy link
Member Author

@swift-ci test macOS

Copy link
Contributor

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

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

This is great. Nice to see all the tests complete in such a short time.
Maybe not for this PR but the system relies a lot on global variables which means there is no chance we can run these tests in parallel. Would be good to look in the future at cleaning up these globals.
Any attempt to support strict concurrency will probably require some work to these globals.

Array("[]".utf8)
}

return HTTPClientResponse(body: .bytes(ByteBuffer(data: Data(payload))))
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to convert to Data to then convert to ByteBuffer

Suggested change
return HTTPClientResponse(body: .bytes(ByteBuffer(data: Data(payload))))
return HTTPClientResponse(body: .bytes(ByteBuffer(bytes: payload)))

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, I've fixed this in the latest commit.

@@ -62,7 +64,11 @@ let package = Package(
),
.testTarget(
name: "SwiftlyTests",
dependencies: ["Swiftly"]
dependencies: ["Swiftly"],
resources: ghApiCacheResources + [
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't know about embedInCode. Nice!

@cmcgee1024
Copy link
Member Author

@swift-ci test macOS

Remove unnecessary resources for the github tags to speed up compilation of the tests

Fix platform use() when given a toolchain version that is not installed
@cmcgee1024
Copy link
Member Author

@swift-ci test macOS

@cmcgee1024 cmcgee1024 changed the title DRAFT: Mock out test interactions with swift.org and github.com Mock out test interactions with swift.org and github.com Jul 22, 2024
@cmcgee1024 cmcgee1024 marked this pull request as ready for review July 22, 2024 18:26
@cmcgee1024
Copy link
Member Author

@swift-ci test macOS

@cmcgee1024 cmcgee1024 merged commit c192812 into swiftlang:main Jul 22, 2024
6 checks passed
@cmcgee1024 cmcgee1024 deleted the refactor-tests branch July 22, 2024 19:12
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