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

initial cli testing frame #341

Merged
merged 10 commits into from May 28, 2019

Conversation

Projects
None yet
2 participants
@MykolaGolubyev
Copy link
Contributor

commented May 22, 2019

Initial DSL for cli testing. Not exposed through WebTauDsl yet. CliJavaTest shows basic java syntax. Approach is similar to Http..

It is most likely going to change so considering it is a private API for now. Want to put it out there so we can start conversation around.

One of the syntax I want to be able to achieve follows

cli.run('command...') {
    on('partial match') {
         http.get( ... ) { 
         }
    }

    onOptional('partial match') {
        http.del(....) {
        }
    }

    output.should contain('blah')
}

Alternatively we may try to use waitTo (not really sure how to approach this)

cli.run('command...') {
    line.waitTo == ~/partial match/ 
    http.get( ... ) { 
    }

    output.should contain('blah')
}

@MykolaGolubyev MykolaGolubyev requested review from tsiq-karold and tsiq-clemens and removed request for tsiq-clemens May 22, 2019

MykolaGolubyev added some commits May 22, 2019

@tsiq-karold

This comment has been minimized.

Copy link
Collaborator

commented May 23, 2019

Can you explain what the desired behaviour of on and onOptional are?

@tsiq-karold
Copy link
Collaborator

left a comment

Do we want to support user supplied environment variables for the process? It's not uncommon for scripts to expect some vars. People may even wish to set a PATH so they can run all the tests without FQN for the scripts they're executing.

Do we want to support multiple script executions? Right now if you do two cli.run calls, they'll have totally separate environments (or shells or whatever you want to call it).

My hunch is yes to the env vars and no to the multiple scripts but curious on your thoughts.

tokenizedMessage(action("running cli command "), stringValue(command)),
() -> tokenizedMessage(action("ran cli clommand"), stringValue(command)),
() -> {
ProcessRunResult runResult = ProcessUtils.run(command.split("\\s+"));

This comment has been minimized.

Copy link
@tsiq-karold

tsiq-karold May 23, 2019

Collaborator

Hmmm, what if someone wants to run foo.sh "something that should be one arg and is therefore in quotes"?

This comment has been minimized.

Copy link
@MykolaGolubyev

MykolaGolubyev May 24, 2019

Author Contributor

good point, thanks

private ProcessUtils() {
}

public static ProcessRunResult run(String[] command) {

This comment has been minimized.

Copy link
@tsiq-karold

tsiq-karold May 23, 2019

Collaborator

I must have written almost identical code at least 10 times in my career :)

This comment has been minimized.

Copy link
@MykolaGolubyev

MykolaGolubyev May 24, 2019

Author Contributor

I feel you :)

new Thread(errorGobbler).start();
new Thread(outputGobbler).start();

process.waitFor();

This comment has been minimized.

Copy link
@tsiq-karold

tsiq-karold May 23, 2019

Collaborator

Do we want to impose some timeouts here?

This comment has been minimized.

Copy link
@MykolaGolubyev

MykolaGolubyev May 24, 2019

Author Contributor

Created issue earlier to track this. Want a skeleton down for now.


process.waitFor();

return new ProcessRunResult(process.exitValue(), outputGobbler.getLines(), errorGobbler.getLines());

This comment has been minimized.

Copy link
@tsiq-karold

tsiq-karold May 23, 2019

Collaborator

Could the reason for your sporadic test failures be that you're not joining the two gobbler threads? They could in theory still be consuming the stream when you call getLines here.

This comment has been minimized.

Copy link
@MykolaGolubyev

MykolaGolubyev May 24, 2019

Author Contributor

Trying

This comment has been minimized.

Copy link
@MykolaGolubyev

MykolaGolubyev May 24, 2019

Author Contributor

You were right, thanks @tsiq-karold. Wonder if executor would have been better in this case.

This comment has been minimized.

Copy link
@tsiq-karold

tsiq-karold May 24, 2019

Collaborator

That thought had occurred to me. However, it won't prevent you from having to join conceptually, instead you'll call get on a future. You'll also have to work out how to size the executor or I suppose let the JVM do that for you and hope it's never an issue (which for this usecase it likely won't be).

@MykolaGolubyev

This comment has been minimized.

Copy link
Contributor Author

commented May 24, 2019

Do we want to support user supplied environment variables for the process? It's not uncommon for scripts to expect some vars. People may even wish to set a PATH so they can run all the tests without FQN for the scripts they're executing.

Do we want to support multiple script executions? Right now if you do two cli.run calls, they'll have totally separate environments (or shells or whatever you want to call it).

My hunch is yes to the env vars and no to the multiple scripts but curious on your thoughts.

Yes for env vars. Not sure I follow multiple scripts part. Do you mean like semi colon separated or an explicit list of commands? I think if there is a good usecase I would take a look at it.

@MykolaGolubyev

This comment has been minimized.

Copy link
Contributor Author

commented May 24, 2019

Can you explain what the desired behaviour of on and onOptional are?

on will fail if there was no match during entire process run. onOptional (better name is welcome) is not going to fail if there was no match, but still lets you react (e.g. send http.delete to clear state).

@tsiq-karold

This comment has been minimized.

Copy link
Collaborator

commented May 24, 2019

Do we want to support user supplied environment variables for the process? It's not uncommon for scripts to expect some vars. People may even wish to set a PATH so they can run all the tests without FQN for the scripts they're executing.
Do we want to support multiple script executions? Right now if you do two cli.run calls, they'll have totally separate environments (or shells or whatever you want to call it).
My hunch is yes to the env vars and no to the multiple scripts but curious on your thoughts.

Yes for env vars. Not sure I follow multiple scripts part. Do you mean like semi colon separated or an explicit list of commands? I think if there is a good usecase I would take a look at it.

Not sure I can think of a use case right now but yes conceptually semi colon separated or && separated is what I was thinking. Generally that'd b the same as executing multiple cli.run sequentially anyway because unless you source the script it'll run in a subshell and therefore won't touch your env.

@tsiq-karold

This comment has been minimized.

Copy link
Collaborator

commented May 28, 2019

The env stuff looks good. Did you want to tackle the args with spaces thing in this PR or create an issue and tackle later? I'm happy to approve if we have an issue for it,

@MykolaGolubyev

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2019

Let's do a separate PR: #346

@MykolaGolubyev MykolaGolubyev merged commit 5d9b660 into master May 28, 2019

4 checks passed

Travis CI - Branch Build Passed
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@MykolaGolubyev MykolaGolubyev deleted the cli-testing branch May 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.