Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

Automatically update AllPackages bundle when regenerating snapshots#123

Merged
liamnichols merged 4 commits intomainCreateAPI/CreateAPI:mainfrom
ln/all-packages-manifest-generationCreateAPI/CreateAPI:ln/all-packages-manifest-generationCopy head branch name to clipboard
Aug 8, 2022
Merged

Automatically update AllPackages bundle when regenerating snapshots#123
liamnichols merged 4 commits intomainCreateAPI/CreateAPI:mainfrom
ln/all-packages-manifest-generationCreateAPI/CreateAPI:ln/all-packages-manifest-generationCopy head branch name to clipboard

Conversation

@liamnichols
Copy link
Member

When we recreate the generator snapshots, if we add a new package we usually have to manually update Tests/CreateAPITests/AllPackages/Package.swift so that CI will try and build the package when we create Pull Requests. This was a pain to keep on top of.

In this change, I update the tests so that they record all of the generated packages and upon completion of the tests, they update the Package.swift file.

It's a little rough and ready, but I didn't want to spend a lot of time refactoring this area although I think that we probably could do. Any suggestions for improvements are welcome.

cc @LePips

@liamnichols liamnichols self-assigned this Aug 7, 2022
@LePips
Copy link
Contributor

LePips commented Aug 7, 2022

Great stuff! With the inclusion of the XCTestObservation conformance do you think we can also delete the Expected folder when snapshots are being generated?

@liamnichols
Copy link
Member Author

Great stuff! With the inclusion of the XCTestObservation conformance do you think we can also delete the Expected folder when snapshots are being generated?

I was thinking about it, but was slightly deterred because of the scenario where you might run just one test rather than all of them. But maybe there is a way to detect that.. I'll keep looking at this because after looking again at the code this morning, I think I want to refactor it some more as well 😄

@liamnichols liamnichols force-pushed the ln/all-packages-manifest-generation branch from 2a12e66 to 6e322dc Compare August 8, 2022 16:05
@liamnichols
Copy link
Member Author

Took a much cleaner approach now by breaking things out into their own responsibilities. We now have PackageCompiler (not sure on that name 😄) to collect the dependencies as we generate the snapshots and then write them out using PackageManifest at the end of the tests

@liamnichols liamnichols merged commit 74da956 into main Aug 8, 2022
@liamnichols liamnichols deleted the ln/all-packages-manifest-generation branch August 8, 2022 16:38
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.