Feat: Skip request

[JorgeTrovisco]

Description

Adds the feature mentioned on #2116. This functionality is important because some endpoints cannot be tested recurrently when we invoke complex APIs. This way we can keep these endpoints in the collection with examples and documentation without running the risk of invoking them in the collection run.

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.

Screenshots

[martinsefcik]

I would probably omit this Skipped OK blue text in brackets for skipped requests. Normally there is HTTP status and skipped request has no HTTP status. So we are mixing apples and pears in this brackets. Also putting OK there doesn't make sense to me as skipping cannot fail. So every skipped request will be always OK.

[JorgeTrovisco]

[sanjai0py]

Hey @JorgeTrovisco, thanks for creating this PR. I will test it and get back to you soon!

[nikischin]

I just checked out your branch locally and run into this issue if I try to use the new function:

Same thing happens if I remove the if before the skip condition, but doesn't happen if I remove the skip function call. Did I do it wrong? If yes I would recommend to implement some intuitive error handling

[nikischin]

What's the reason item.responseReceived is set here?
Seems like a copy paste mistake but I could be wrong.
I would expect action.payload.responseReceived to be undefined when the request is skipped so item.responseReceived would be as well?

Also why are we using .find() here, whereas all the other types use .findLast()?

[JorgeTrovisco]

Hi,

You're right, responseReceived is no longer necessary.

Regarding the find vs findLast change, it was made after I checked out the branch, so I didn't catch it during the merge.

[nikischin]

Thank you so much for your fast reply! Looks very good (on code level)

So have you been able to reproduce my error or do I need to set up some collection to reproduce the error?

Or did I use the function not as intended?

[JorgeTrovisco]

I just checked out your branch locally and run into this issue if I try to use the new function: Same thing happens if I remove the if before the skip condition, but doesn't happen if I remove the skip function call. Did I do it wrong? If yes I would recommend to implement some intuitive error handling

I can't reproduce the error. Can you share your collection?

bru.skipRequest();

If I use the skipRequest on the current bruno I receive the following

[martinsefcik]

Shouldn't it be also implemented for bruno-cli ?

[nikischin]

So this is the smallest reproducible example I was able to build:

example.bru

meta {
  name: example
  type: http
  seq: 4
}

get {
  url: https://api.restful-api.dev/objects
  body: none
  auth: none
}

script:pre-request {
  if(!bru.getEnvVar("sampleVar")){
    bru.skipRequest();
  }
}

For reproduction it is the easiest to set up a new ENV with the var sampleVar set to some random value. Whenever you have selected this environment, the request will go through without being skipped, and when you select "No Environment" it will fail with the error provided.

[JorgeTrovisco]

The skip request is just for collection running, so when you execute the request manually it should still be executed.

It works without the creation of the environment variables? Is the error related with trying to get a unavailable variable?

[nikischin]

The skip request is just for collection running, so when you execute the request manually it should still be executed.

It works without the creation of the environment variables? Is the error related with trying to get a unavailable variable?

It seems not to be related to getting an undefined environment variable.

In the published version of bruno (without the skip function implemented) it just executes the request and does get the response as usual (so does nothing regarding skipping obviously).

(As you can see I don't have any environment selected so the variable cannot be defined).

Also being able to use the skip function only for the runner in my opinion is not intuitive. This is definitely subject to discussion but I would expect it to also skip on standalone run. As you can see there might be many reasonable examples why you might want to skip a run and in my opinion it doesn't make a difference if it's from within the runner or standalone.

I personally would even implement a condition to skip any call (pre request script on collection level) whenever some auth token is not set or something, as it is not beneficial to even run requests I know for sure that would not run or would not be beneficial to test or might even cause other problems.

[JorgeTrovisco]

Still no luck at reproduce the error, however I found a bug that exists on main

The first element of the list does not have the OK after 200. Should I open an issue to fix this error?

[JorgeTrovisco]

Also being able to use the skip function only for the runner in my opinion is not intuitive. This is definitely subject to discussion but I would expect it to also skip on standalone run. As you can see there might be many reasonable examples why you might want to skip a run and in my opinion it doesn't make a difference if it's from within the runner or standalone.

• Using that approach, I'd have to remove the skip request each time I want to test the endpoint, then add it back when I run the collection.
• If I don't want to run a standalone request, I simply won't press the "send" button.

[nikischin]

Still no luck at reproduce the error, however I found a bug that exists on main

The first element of the list does not have the OK after 200. Should I open an issue to fix this error?

Have you tried executing the single request I provided in my example (not from within the runner) by just using the arrow button on the top right?

Regarding your finding, feel free to open a bug with a reproducible example. I was able to reproduce it with one of my collections but not in another. So this seems to happen only for some cases.

[JorgeTrovisco]

I still haven't had any luck reproducing that behavior.

I used your .bru file and set up the environment variables. It worked successfully both with and without the environment, and I was able to skip the request during the collection run based on the environment variable.

[nikischin]

Also being able to use the skip function only for the runner in my opinion is not intuitive. This is definitely subject to discussion but I would expect it to also skip on standalone run. As you can see there might be many reasonable examples why you might want to skip a run and in my opinion it doesn't make a difference if it's from within the runner or standalone.

• Using that approach, I'd have to remove the skip request each time I want to test the endpoint, then add it back when I run the collection.
• If I don't want to run a standalone request, I simply won't press the "send" button.

What you are saying is actually a very valid point. I think to break it down, there are two different requirements behind it.

Your Requirement: (I suppose)

• Skip Requests in Runner, because you wouldn't want to have them in the flow but would like to have them in your collection. Like let's say you have most requests you would do some "automated Testing", while keeping others in the collection for manual run.

My Requirement:

• Conditionally Skip Requests, when some preconditions are not met. Can have different reasons, like authentication missing or variables are not meeting some minimum quality gates. (Like I can only delete a entry, if I created one before)

However, honestly, I feel like the implementation you did with the bru.skipRequest() I would expect to meet my requirement. While for yours I would expect either to have a option in the runner to select the requests which you would like to run with some checkboxes or something or to introduce another function like bru.isRunner() to determine if the current call is executed within a runner. You could then use the bru.skipRequest() like this:

if(bru.isRunner()){
  bru.skipRequest();
}

In any case we both agree, that, if you execute a single request which is using the bru.skipRequest(); it should not run into an error and either skip the request (my expectation) or just run it (your expectation). If you cannot reproduce it it might be caused by my local installation, so if it is running in @sanjai0py 's installation or on some other machines without errors as well I guess it should be fine!

Does this make sense? Do you agree? Thank you so much already for all your efforts, I really appreciate your work!

[JorgeTrovisco]

• Adjust the screen of the single call result to represent the skipped request;

I think that is good at this point

• Understand and fix the error in safe mode;

Already fixed!

• Understand how can this work in CLI without the need to explicit implementation that needs to be addressed in another PR (Would appreciate some help)

Is there any guide like https://github.com/usebruno/bruno/blob/main/contributing.md to run the CLI version?

[nikischin]

I haven't tested in cli but in UI it works very well for both runner and single request, thank you so much!

[mangelco]

Thank you for your work on this! This feature is essential for our current project. Implementing this functionality will greatly enhance endpoint testing, and I believe many other users will benefit from it as well

[DumitruAndreea]

Hi! When this future will be available?

[nikischin]

Still no update on this @helloanoop @lohxt1 ? I would love to further collaborate to the project, but waiting for over 6 months for a PR with no progress or feedback feels annoying sorry.

[JorgeTrovisco]

Really, I was also very excited to contribute to this project, but the effort put into a feature that I really needed with no return was very frustrating. I didn’t even resolve the conflicts because I was expecting some type of feedback.

[tlaloc911]

There are many great PRs just filling with cobwebs. In my case I've incorporated changes of you both and work great. I thing community are already testing and validation this kind of changes so, code quality shouln't be an issue for merging

Lack of feedback only causes fragmentation and stops the willing of collaborating.

[nikischin]

Since there's no update still and there is no progress, and for me personally this feature would be more important than any other updates from the recent months I was wondering if we could just merge and build this version? I would be fine with not receiving updates from the main version and keeping this fork.

I have no experience in how the bundling works and if this can be done easily, but maybe we could give it a try, what do you think @JorgeTrovisco ?

[JorgeTrovisco]

I was about to merge with the current version when I came across this PR (#3719). I haven’t had the chance to review its implementation yet, but it seems to cover this feature. Therefore, I believe there’s no need to proceed with the one I was working on.

[JorgeTrovisco]

I had the opportunity to review its implementation, and while it addresses the need to skip the request, it unfortunately results in the loss of features.

We can add bru.runner.skipRequest(); in the pre request script and the request is skipped on the runner

However:

• Shows the skip request as failed but doesn't count it at failed
• We lost the ability to skip single request

Feature	This PR approach	3719 approach
Representation of skipped requests
Count of skipped requests		N/A (Its not counted as anything)
Skip single request		N/A (As far as I can see)

I believe we can proceed with the #3719 version, as it addresses the issue of skipping requests on the runner and it is already merged.

@lohxt1 When can we expect a version 1.38.0 with this feature included?

[nikischin]

I had the opportunity to review its implementation, and while it addresses the need to skip the request, it unfortunately results in the loss of features.

We can add bru.runner.skipRequest(); in the pre request script and the request is skipped on the runner
[...]
However:

• Shows the skip request as failed but doesn't count it at failed
• We lost the ability to skip single request

Hi @JorgeTrovisco thank you soo much again!

And well this is actually kinda great news! For me the features added in #3719 add a lot of value to Bruno. However, I personally preferred your implementation of the request skipping (especially those diffs you pointed out). I personally would wanna see how many requests were skipped and would like to skip a single request if certain pre-conditions are not met. (This avoids generating error logs on my server)

I however don't understand why there was not any Feedback on this thread @lohxt1 and you just created a new PR taking the chance of @JorgeTrovisco to have a contribution to the project. Neither this PR nor the original feature request are mentioned in your PR. This gives the feeling community contributions are not appreciated, as the something similar already happened with #3105 and most likely other community created PRs. Especially seeing that the good ideas implemented with this PR seem to not taken into consideration for the merged PR.

[nikischin]

As the skipping was released with the latest version I gave it a try. It does a decent job, with the tradeoffs @JorgeTrovisco mentioned in mind. In order to keep track of the missing features of the current released version I opened two new feature requests. One Covering the single request skipping #3764 and one covering the UI improvements #3765.

While I personally don't see much chance for an implementation covering #3764 done by the community to be merged, I was wondering if we would want to give the points mentioned in #3765 a try at least. For my impression this should be done easily without too much of an effort.

I could even migrate your implementation details to a new branch with the current head. However, as I would do this in a new branch, I would not be able to keep the history and kinda "steal" your contribution @JorgeTrovisco. As you put in all the work already, it would feel more fair to have it merged with your name on it. So let me know if you would like to give this a try maybe or if you would prefer me doing that for you.

[JorgeTrovisco]

@nikischin Yes, the points mentioned in #3765 should be relatively straightforward to implement even with the current version. However, given the possibility that the implementation might be ignored or replaced with similar code without any mention, I don’t think I’ll pursue it myself. That said, feel free to implement it. I’d be happy to review and validate it afterward.

[helloanoop]

Hi @JorgeTrovisco,

First, I’d like to sincerely apologize for not addressing your PR in a timely manner. It was absolutely not our intention to overlook your contribution.
We truly value the time and effort you’ve put into the PR and the active discussions.

We recently had to build utility functions - bru.runner.stopExecution(), bru.getTestResults(), bru.getAssertionResults().
And the bru.runner.skipRequest() naturally made sense to be added to the runner and we missed looking at this PR.

We had a team meeting internally around this hiccup and we deeply regret how this situation unfolded (given the discussions and time spent by the community in shaping up this feature by you and @nikischin)

@JorgeTrovisco We've gone ahead and attributed credits for your work in the release changelog

As we perused through the PR and comments, we noticed there are additional enhancements in your work (as described in #3764 and #3765) that were not included in the version we released.
If you’re willing, we’d love for you to create a new PR focusing on those extra features, and we’ll prioritize its review.

Thank you @nikischin for helping shape this PR and helping with creating follow-up tickets to track the additional enhancements.

Again, we deeply regret how this situation unfolded here and are committed to doing better.
Thank you for your understanding and for being such an active and engaged member in Bruno community.

Thanks,
Anoop