feat: bru.setNextRequest() #619

mj-h · 2023-10-16T15:30:50Z

Description

This PR adds the method bru.setNextRequest(requestName). It can be used in pre- and postrequest scripts (last write wins). It works in the collection-runner and in the CLI.

Closes #534.

Contribution Checklist:

The pull request does not introduce any breaking changes
I have read the contribution guidelines.
Create an issue and link to the pull request.

Usage::

Result in the UI Collection Runner

Result in the CLI

helloanoop · 2023-10-18T17:18:05Z

@mj-h Some questions

Can you share a real world scenario where the setNextRequest() api become useful.
Should we consider supported full paths like setNextRequest("folder/get users")

mj-h · 2023-10-18T18:18:44Z

Sure!

Typical real-world scenarios for setNextRequest() are:
- Async service with polling pattern: For these services, you send "start-process" / "get-status" / "verify-final-result" requests. The "get-status" request needs to be repeated until the status changes, or until a maximum time / number of requests has been reached. We use this pattern a lot internally, and it's a common pattern for loading results of a slow process into the UI.
- Service-creation with cleanup: Often, you find the "create-service" / "test-service" / "destroy-service" pattern. When one of the tests fail, you often want to jump straight to "destroy-service".
That's an interesting idea. I'm definitely open to it, and it would reduce the chance of getting annoying name-collisions -- but it increases the chance for breaking things, when someone renames a folder without checking the code. Tomorrow, I'll ask a few colleagues whom I know to be using .setNextRequest heavily. I would tend to make the foldername optional -- if it's there, it must be matched, if it's not there, it's not part of the matching-logic.

I truly appreciate your thoughts here. The .setNextRequest()-feature keeps me up at night, and I'm not sure I am doing humanity a favor when implementing it. The worst Postman collections I had to maintain were collections that made heavy use of .setNextRequest and variables, which are really just GOTOs and global variables. Not fun. But on the other hand, Postman was the only test-tool that supported async APIs.

mj-h · 2023-10-18T18:40:07Z

Two thoughts on how .setNextRequest() could be made more palatable:

Similar to cmd + enter, we could have a keyboard shortcut cmd + shift + enter that sends a request and goes to the "next" request, just like the collection runner would. That way you could step through a collection in a way that Postman never could. This is the main debugging feature I missed in Postman. (And for people who don't use .setNextRequest, it's still a convenience feature that allows them to run collections top-to-bottom more quickly).
Like hurl is discussing it, we could ave skip and repeat blocks in the bru file. Something like:
```
skip-if {
  someOtherVar != "some-value"
}

repeat-if {
  someVar == "not-ready" 
}
```
Then the skip / repeat information could be placed more prominently in the UI (like the assertions), and we could do safe renaming of requests and folder. But the bru language would become bigger and bigger...

bdentino · 2023-10-18T19:17:00Z

Chiming in with another real-world use case I would love to see supported before I can fully migrate my workloads from postman:

A common pattern in our automation tests & cache-warming flows are to make a request to one endpoint which provides a list of available resources, and then issue a different request for each resource returned. So for example I will have a postman collection with 2 requests: GET /foos and GET /foos/:fooid. The first request is expected to return a list of n fooids, and I then I'll issue GET /foos/:fooid n times, while updating the value of fooid via post-request scripts during the run.

I agree with @mj-h that setNextRequest feels a bit "unclean" from a design pov. Not just because of the GOTO/globalish nature of it, but also because it forces me to modify collection variables during a run in order to achieve flows like this and it feels wrong to have the collection end up in a different state after any individual run.

Another approach which would enable my use-case, and presumably others, would be to inject a function you can call from scripting contexts to 'queue' additional requests. I suppose it would only make sense in the context of a collection run as opposed to executing a single request from the ui (then again, so does getNextRequest). But I'm thinking it could look something like this:

// post-request script

// run is a new global variable, alongside `req`, `res`, `bru`, etc...
// requests is a list of requests defined in the running collection
// queue is a list of pending requests waiting to run
const { requests, queue } = run

// Find the request you want to queue. Solves 'globalness' problem 
// in that you can only queue requests which you know exist. Does 
// limit you to requests within the collection, though so no reaching 
// out to other collections/folders (personally i think that's a good 
// thing for maintainability/cohesion but could be convinced 
// otherwise).
const newRequest = requests.find((r) => r.name === 'Get Foo')

// Create an instance of this request that will execute with specific
// variables that would supersede any pre-defined collection variables
// (solves the problem of having to modify collection state during run)
const toRun = newRequest.withVars({ ...overrideVars })

// Run new request last (after other queued requests)
queue.push(toRun)

// Run new request next (before other queued requests)
queue.unshift(toRun)

What do you think?

mj-h · 2023-10-19T12:15:04Z

I talked to colleagues, and I checked usage on various GitHubs. My findings:

on public GitHub, only 2% to 3% of collections use postman.setNextRequest.
on our Enterprise GitHubs (yes, several GitHubs, because $enterprise...), about 20% to 25% of the Postman collections use postman.setNextRequest (!)
- half of those usages are postman.setNextRequest(null) or postman.setNextRequest() to abort the collection run. Most times, the author actually wants to exit with error. On the command line, they could have used newman run --bail, but the UI collection runner has no such feature (afaik).
  - Idea: "stop-on-error" should be a collection-level property that is respected by the CLI and the UI collection runner.
- a third of usages are postman.setNextRequest(request.name) for a do-while loop, mostly to check the results of async APIs. It is occasionally used as generic retry mechanism.
- the remaining usages seem to be a hodgepodge of "skip next request if previous response is foo" and "goto error-handling-part".
- one clear misuse I saw was an author who painstakingly used postman.setNextRequest("foo") on every request to jump to the next request, because he did not know that that's the default behavior of the collection runners. But that was an outlier and should not influence our design.
no user I spoke to had a clear preference of .setNextRequest vs skip+repeat.

@bdentino: I did not encounter your scenario in our collections, but I did not check every collection in detail. In any case, I would say it's a rare (but still valid!) usage. Your proposal, however, boils down to a goto X then goto Y then goto Z statement, which is powerful, and which makes your usecase easier, but which is harder to use and harder to reason about than a simple goto.

mj-h · 2023-10-19T12:28:25Z

My proposal going forward:

let's include bru.setNextRequest() in the v1 release, to enable more users to switch to bruno. Use similar semantics as in postman. Clearly mark it as "experimental / not recommended" in the documentation.
let's think about better control-flow logic for the v2 release. I think a strong case can be made for skip and repeat, but making it ergonomic takes some more thinking and tinkering.
possibly mark bru.setNextRequest() as deprecated in the future.

mj-h · 2023-10-24T20:44:57Z

Merged main and confirmed that it still works in UI and CLI. To be discussed: How to handle the "request name not found" case. Postman exited early here; with exit code 0, I believe. For easier compatibility, we could do the same.

bdentino · 2023-10-24T20:49:57Z

@mj-h I found some UI problems in my own testing and created a PR with fixes here: mj-h#1

mj-h · 2023-10-26T21:11:54Z

While testing this further, I noticed that there is no way to stop the collection runner once it started. But that's a separate issue (#143).

helloanoop · 2023-10-26T21:38:53Z

@mj-h Thank you for leading the discussion around this.

I'd like to spend some more time on this. Been busy with many other things. I will put some dedicated time towards this issue the coming week.

PS: See v1.0.0 discussion

mj-h · 2023-11-01T09:37:51Z

Sure, happy to help! In the meantime, I'll add two missing behaviors:

setNextRequest(null) ~~and setNextRequest(undefined)~~ should break the the execution loop (no error)
there should be a maximum number of loop iterations to prevent truly infinite loops. Other popular products use N=10000. I think that's a reasonable choice.

Update: Behaviors added. They don't make the code prettier or the documentation easier -- but I still think they are worth it.

This is especially useful for the bru cli, to make sure that test-runners that are accidentally stuck in an infinite loop still terminate in a reasonable amount of time and don't hog up resources.

fpassaniti · 2023-11-06T12:10:09Z

For my usages, this feature is a mandatory prerequisite to move from Postman to Bruno, as it's a pillar to all my collections.
I need to dynamically, depending on file inputs, or my API results, to switch from a request to another, or even loop over the same request several time in my collection.
I intensively use postman.setNextRequest to chain, loop, or even stop my collection dynamically.
I look forward to see this function released, as for now I'll be force to put on hold our migration to Bruno without it.
Thanks

fpassaniti · 2023-11-06T12:12:06Z

packages/bruno-cli/src/commands/run.js

+      const nextRequestName = result?.nextRequestName;
+      if (nextRequestName !== undefined) {
+        nJumps++;
+        if (nJumps > 10000) {


Shouldn't it be a parameter in bruno.json (the max number of jumps allowed) ? just in case

I thought about it -- but there are already a ton of config options... Do you have a use-case that needs it? If so, we can add it. If not, I would say YAGNI.

YAGNI it is then ;)

mj-h · 2023-11-23T08:58:16Z

@helloanoop : Any news on if and/or when this can be included? I'm under pressure to migrate our API-tests away from newman, and I can stall my org for maybe one or two more weeks...

helloanoop · 2023-11-24T10:37:58Z

@mj-h This will be included. I will spend some time on this and give you an update over this weekend.
Thank you for your patience.

helloanoop · 2023-12-05T20:13:15Z

Thank you @mj-h for your work on this !
Everyone, Thank you for your patience.

helloanoop · 2023-12-05T21:19:17Z

Its live 🎊

GUI - v1.4.0
CLI - v1.2.0

CoenraadS · 2023-12-06T08:51:01Z

I just tried it, but I couldn't get it to work?

I tried

bru.setNextRequest("Hacker News/GetById");

bru.setNextRequest("GetById");

In GetById I have in the pre-request script a console.log("Hello World") but I never see it,

mlestoquoy · 2023-12-06T18:56:33Z

First, thanks for this feature. I managed to make it work from bruno, but not from CLI:

works with bruno 1.4.0 (Run):

and not with bru 1.2.0 (bru.setNextRequest is not a function):

Did I miss something ?

helloanoop · 2023-12-06T19:29:15Z

@mlestoquoy I just released a patch fix for this in bru cli v1.2.1

Can you install it and check if this issue is resolved ?

mlestoquoy · 2023-12-06T22:47:00Z

@mlestoquoy I just released a patch fix for this in bru cli v1.2.1

Can you install it and check if this issue is resolved ?

Yes it works !
Thanks @helloanoop

feat: bru.setNextRequest()

d4c0207

jhoppenstedt approved these changes Oct 16, 2023

View reviewed changes

helloanoop added this to the v2 milestone Oct 18, 2023

Timwun approved these changes Oct 19, 2023

View reviewed changes

Merge branch 'main' into feature/add-bru-setNextRequest

3374db1

mjhcorporate and others added 2 commits October 26, 2023 22:34

Merge branch 'main' into feature/add-bru-setNextRequest

4a1d45f

Fixes for getNextRequest in UI

d76253e

mjhcorporate added 3 commits November 1, 2023 22:59

Merge branch 'main' into feature/add-bru-setNextRequest

db0de68

Count the number of jumps to break out of infinite loops.

129d659

This is especially useful for the bru cli, to make sure that test-runners that are accidentally stuck in an infinite loop still terminate in a reasonable amount of time and don't hog up resources.

feat: bru.setNextRequest(null) breaks execution without error

8183ce0

fpassaniti reviewed Nov 6, 2023

View reviewed changes

CoenraadS mentioned this pull request Nov 19, 2023

[Feature Request] Programmatically get request name #987

Closed

helloanoop merged commit 3061507 into usebruno:main Dec 5, 2023

helloanoop mentioned this pull request Dec 17, 2023

[BUG] Collection Runner not performing tests nor assertions #1215

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: bru.setNextRequest() #619

feat: bru.setNextRequest() #619

mj-h commented Oct 16, 2023 •

edited

Loading

helloanoop commented Oct 18, 2023

mj-h commented Oct 18, 2023 •

edited

Loading

mj-h commented Oct 18, 2023

bdentino commented Oct 18, 2023

mj-h commented Oct 19, 2023

mj-h commented Oct 19, 2023

mj-h commented Oct 24, 2023

bdentino commented Oct 24, 2023

mj-h commented Oct 26, 2023

helloanoop commented Oct 26, 2023

mj-h commented Nov 1, 2023 •

edited

Loading

fpassaniti commented Nov 6, 2023

fpassaniti Nov 6, 2023 •

edited

Loading

mj-h Nov 6, 2023

fpassaniti Nov 6, 2023

mj-h commented Nov 23, 2023

helloanoop commented Nov 24, 2023

helloanoop commented Dec 5, 2023

helloanoop commented Dec 5, 2023

CoenraadS commented Dec 6, 2023 •

edited

Loading

mlestoquoy commented Dec 6, 2023

helloanoop commented Dec 6, 2023

mlestoquoy commented Dec 6, 2023

feat: bru.setNextRequest() #619

feat: bru.setNextRequest() #619

Conversation

mj-h commented Oct 16, 2023 • edited Loading

Description

Contribution Checklist:

helloanoop commented Oct 18, 2023

mj-h commented Oct 18, 2023 • edited Loading

mj-h commented Oct 18, 2023

bdentino commented Oct 18, 2023

mj-h commented Oct 19, 2023

mj-h commented Oct 19, 2023

mj-h commented Oct 24, 2023

bdentino commented Oct 24, 2023

mj-h commented Oct 26, 2023

helloanoop commented Oct 26, 2023

mj-h commented Nov 1, 2023 • edited Loading

fpassaniti commented Nov 6, 2023

fpassaniti Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

mj-h Nov 6, 2023

Choose a reason for hiding this comment

fpassaniti Nov 6, 2023

Choose a reason for hiding this comment

mj-h commented Nov 23, 2023

helloanoop commented Nov 24, 2023

helloanoop commented Dec 5, 2023

helloanoop commented Dec 5, 2023

CoenraadS commented Dec 6, 2023 • edited Loading

mlestoquoy commented Dec 6, 2023

helloanoop commented Dec 6, 2023

mlestoquoy commented Dec 6, 2023

mj-h commented Oct 16, 2023 •

edited

Loading

mj-h commented Oct 18, 2023 •

edited

Loading

mj-h commented Nov 1, 2023 •

edited

Loading

fpassaniti Nov 6, 2023 •

edited

Loading

CoenraadS commented Dec 6, 2023 •

edited

Loading