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

read specs args from stdin pipe #2061

Closed
wants to merge 9 commits into from
Closed

Conversation

sriv
Copy link
Member

@sriv sriv commented Jul 29, 2021

No description provided.

fixes #1991

Signed-off-by: sriv <srikanth.ddit@gmail.com>
@sriv sriv requested a review from zabil July 29, 2021 11:08
@sriv sriv marked this pull request as draft July 29, 2021 13:03
sriv added 3 commits July 30, 2021 15:51
Signed-off-by: sriv <srikanth.ddit@gmail.com>
Signed-off-by: sriv <srikanth.ddit@gmail.com>
Signed-off-by: sriv <srikanth.ddit@gmail.com>
@sriv sriv marked this pull request as ready for review August 4, 2021 10:11
Signed-off-by: sriv <srikanth.ddit@gmail.com>
Signed-off-by: sriv <srikanth.ddit@gmail.com>
Signed-off-by: sriv <srikanth.ddit@gmail.com>
Signed-off-by: sriv <srikanth.ddit@gmail.com>
@@ -39,7 +41,8 @@ var (
GaugeCmd = &cobra.Command{
Use: "gauge <command> [flags] [args]",
Example: ` gauge run specs/
gauge run --parallel specs/`,
gauge run --parallel specs/
cat run.txt | gauge run # run.txt contains list of specs to execute`,
Copy link
Member

Choose a reason for hiding this comment

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

Will this work?

gauge run  < run.txt

That's probably the semantic way of passing run list.

Copy link
Member Author

@sriv sriv Aug 5, 2021

Choose a reason for hiding this comment

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

I am now having second thoughts on this idea. This implementation is getting way too complex for what it serves.

Some things I realized:

  • getting pipes to work cross platform is possible, but the semantics are different so there will be a learning curve for users.
  • if stdin is not a tty, it does not mean that data is getting piped in! ex - maven/gradle plugins launch gauge as a subprocess, and this will make the stdin non-tty. So the only way to know that stdin is a pipe is to get the file descriptor. Again, this can be tricky across platforms (at least I do not know of a reliable way to do this).
  • The IDE plugins also invoke gauge run. If there is ever a requirement to pipe in data, this can get complicated.

I am now considering that we just do the file approach, having support for something like gauge run -f run.json where the run.json is basically a serialized args list.

Thoughts @zabil?

Copy link
Member

Choose a reason for hiding this comment

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

Yes that's right, for cross compatibility and working with build tools like maven, gradle and npm it makes sense to provide this as file option.

I also feel it's useful to have this per environment and convention in the env folder like args.json

@sriv
Copy link
Member Author

sriv commented Aug 21, 2021

Closing this. We should use a file to allow this instead. See conversation above.

@sriv sriv closed this Aug 21, 2021
@haroon-sheikh haroon-sheikh deleted the 1991_read_args_from_stdin branch June 26, 2023 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants