Skip to content
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

Handle multiple flow files in test command #1995

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

tokou
Copy link
Contributor

@tokou tokou commented Aug 31, 2024

Proposed changes

  • I opted for a space separated list as it's an argument and not an option
  • I only affected test command and tried to have no effect on other commands

Testing

  • Added test cases in WorkspaceExecutionPlanner

Issues fixed

Fixes #1990

@bartekpacia
Copy link
Contributor

Thanks, this looks nice!

I'm curious if tab-completion works when the user wants to pass a few space-delimeted yaml files?

@tokou
Copy link
Contributor Author

tokou commented Aug 31, 2024

Yes it works

@bartekpacia
Copy link
Contributor

bartekpacia commented Aug 31, 2024

Looks like test failed because [email protected] was entered instead of [email protected].

screenshot-❌-1725115883738-(Fill out form)

Weird issue, I assume it's some random flakiness. I skimmed over existing bugs but didn't find an existing issue for it.

Action taken:

@bartekpacia
Copy link
Contributor

bartekpacia commented Sep 3, 2024

@tokou there's a problem now because of withDefaultShellEnv, after we've merged #1945

# Conflicts:
#	maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt
#	maestro-orchestra/src/main/java/maestro/orchestra/workspace/WorkspaceExecutionPlanner.kt
#	maestro-orchestra/src/test/java/maestro/orchestra/workspace/WorkspaceExecutionPlannerErrorsTest.kt
#	maestro-orchestra/src/test/java/maestro/orchestra/workspace/WorkspaceExecutionPlannerTest.kt
@bartekpacia
Copy link
Contributor

LGTM, thanks!

@bartekpacia bartekpacia merged commit 8cd7387 into mobile-dev-inc:main Sep 4, 2024
6 checks passed
@bartekpacia bartekpacia mentioned this pull request Sep 4, 2024
@tokou tokou deleted the accept-multiple-flows branch September 4, 2024 13:30
@tokou tokou mentioned this pull request Sep 7, 2024
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.

[Feature Request] Accept comma separated list of flows or flow directories
2 participants