Skip to content

Conversation

neonichu
Copy link
Contributor

Printing the list needs to continue beyond the first non-command plugin.

resolves #6804

Printing the list needs to continue beyond the first non-command plugin.

resolves #6804
@neonichu neonichu self-assigned this Aug 14, 2023
@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@tomerd
Copy link
Contributor

tomerd commented Aug 14, 2023

add test?

@neonichu
Copy link
Contributor Author

add test?

I guess that would make sense, but I am reluctant to add more integration tests here. We should refactor this entire thing so it becomes testable.

@neonichu
Copy link
Contributor Author

I am going to try if making SwiftTool take a file system parameter may help here. It's still a bit too broad for my taste, but might at least enable us to write tests that aren't as slow and unmaintainable as the current command tests.

@neonichu
Copy link
Contributor Author

I am going to try if making SwiftTool take a file system parameter may help here. It's still a bit too broad for my taste, but might at least enable us to write tests that aren't as slow and unmaintainable as the current command tests.

This works, but the test is still extremely slow (~8s when using a very basic package).

@neonichu
Copy link
Contributor Author

neonichu commented Aug 17, 2023

It seems like the slowness is mostly the result of excessive and preemptive shelling out. I wonder if we could come to a model where we would instead do that on demand. In any case, I don't think I can solve that here and I'll add my test as-is.

@neonichu
Copy link
Contributor Author

Ah, of course --list uses print directly :/

@neonichu
Copy link
Contributor Author

Going to merge this as-is as discussed offline, #6827 will stay open to improve test infra and add test coverage for plugin --list.

@neonichu neonichu merged commit f65e3fc into main Aug 25, 2023
@neonichu neonichu deleted the fix-plugin-list branch August 25, 2023 05:27
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.

Command plugin can't run from CLI
2 participants