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

Implement detection for runner #3105

Closed
wants to merge 3 commits into from

Conversation

nikischin
Copy link

@nikischin nikischin commented Sep 14, 2024

Description

Implement a new function bru.getExecutionMode() to use inside scripting. Returns single, runner or cli.

Fixes #3104
Related to #2397
Includes changed from #1875 partially

Thank you @JorgeTrovisco this is a modified implementation of your PR. Thank you also @tlaloc911 as this also includes your changes.

image

The real benefit of this PR will be implemented with the merging of #2397, as this functionality has been extracted from this PR in order to keep it focussed on one feature. With both this and #2397 merged, you could do something like this in order to skip requests on runner runs but still have them for single tests.

if(bru.getExecutionMode() === 'runner'){
   bru.skipRequest();
}

Or you could even do something like this

if(bru.getVar("myAuthKey") === undefined){
  bru.skipRequest();
  
  if(bru.getExecutionMode() === 'runner'){
     bru.setNextRequest("getAuthToken");
  }
}

Contribution Checklist:

  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.

Publishing to New Package Managers

Please see here for more information.

@nikischin nikischin mentioned this pull request Sep 14, 2024
5 tasks
@tlaloc911
Copy link
Contributor

In fact, there are three execution modes: single, runner and also CLI, some time ago I created this PR #1875

@nikischin
Copy link
Author

In fact, there are three execution modes: single, runner and also CLI, some time ago I created this PR #1875

This is indeed a very valid point. I am happy to modify my implementation or you are also welcome to modify yours.

I don't think saving the execution mode to a normal variable is a preferred way as this might conflict with other user defined variables and also is mixing context. So I would recommend something like this:

bru.getExecutionMode()

with return single, runner or cli.

What do you think @tlaloc911 @JorgeTrovisco

@JorgeTrovisco
Copy link

bru.getExecutionMode()

This approach require to have some documentation attached (explaining what would be the expected output)

Does Bruno already have global constants? Would be a great approach to have something like (bru.getExecutionMode() == ExecutionMode.Single)

@tlaloc911
Copy link
Contributor

@nikischin , I agree, using variables is not a clean solution. I did that for an immediate need on that time. Your solution is better and I hope it gets merged

@nikischin
Copy link
Author

I implemented the new variant with the help of your PR #1875 @tlaloc911

Could you please test it for me, especially carefully test the cli variant, as I am usually not using cli.

@tlaloc911
Copy link
Contributor

I implemented the new variant with the help of your PR #1875 @tlaloc911

Could you please test it for me, especially carefully test the cli variant, as I am usually not using cli.

sure!, thanks

@JorgeTrovisco
Copy link

Hi @nikischin, works great in developer mode

Captura de ecrã 2024-09-14, às 22 34 39
Captura de ecrã 2024-09-14, às 22 36 00

In Safe mode is throwing a error only for the runner
Captura de ecrã 2024-09-14, às 22 39 33

In the single request is working fine
Captura de ecrã 2024-09-14, às 22 40 37

Seems related to the error i was facing with the skipRequest, can you reproduce this behavior?

@nikischin
Copy link
Author

In Safe mode is throwing a error only for the runner

Captura de ecrã 2024-09-14, às 22 39 33

Seems related to the error i was facing with the skipRequest, can you reproduce this behavior?

Ah damn, I will have a look. Thank you so much for the feedback, really appreciate it!

@JorgeTrovisco
Copy link

JorgeTrovisco commented Sep 14, 2024

I was unable to fix this, however i think that we are missing something arround the packages/bruno-js/src/sandbox/quickjs/shims/bru.js, I added the function and the error no longer occurs, however the function does not work as expected

In safe mode the code is executed in a secure sandbox, because of that we need to declare all the functions in the shims file packages/bruno-js/src/sandbox/quickjs/shims/bru.js

Be sure to shutdown the current instance and run the commands

npm run build:graphql-docs && npm run build:bruno-query && npm run build:bruno-common && npm run sandbox:bundle-libraries --workspace=packages/bruno-js

and then run again

@nikischin
Copy link
Author

In safe mode the code is executed in a secure sandbox, because of that we need to declare all the functions in the shims file packages/bruno-js/src/sandbox/quickjs/shims/bru.js

Ah great, thank you so much, I just found this out myself before reading your comment.

Be sure to shutdown the current instance and run the commands

npm run build:graphql-docs && npm run build:bruno-query && npm run build:bruno-common && npm run sandbox:bundle-libraries --workspace=packages/bruno-js

and then run again

This part, I don't understand. What do I need it for?

@JorgeTrovisco
Copy link

This part, I don't understand. What do I need it for?

To rebuild all the packages. Mine didn't work until I've done that

@helloanoop
Copy link
Contributor

Hey @nikischin

I feel we should not use runtimeVariables to store this info.
Also, I think standalone mode is a better term instead of single.

Can you review @lohxt1 PR #3200

@nikischin
Copy link
Author

Great, thanks @helloanoop @lohxt1! Closing this ticket in favor of #3200

@nikischin nikischin closed this Sep 26, 2024
@tlaloc911 tlaloc911 mentioned this pull request Oct 14, 2024
5 tasks
@tlaloc911
Copy link
Contributor

I implemented the new variant with the help of your PR #1875 @tlaloc911
Could you please test it for me, especially carefully test the cli variant, as I am usually not using cli.

sure!, thanks

sorry for the very late response, cli worked as expected, thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Detection for runner in script
5 participants