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

feat: show full request url in RunIt response #906

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jkaster
Copy link
Contributor

@jkaster jkaster commented Nov 11, 2021

The Response tab now shows the full request URL, not including the request body (which can't be included in this scenario)

Screenshots

image

image

image

The Response tab now shows the full request URL, not including the request body (which can't be included in this scenario)
@github-actions
Copy link
Contributor

APIX Tests

    1 files    77 suites   3m 26s ⏱️
322 tests 309 ✔️ 13 💤 0 ❌
332 runs  319 ✔️ 13 💤 0 ❌

Results for commit 7f48a16.

@github-actions
Copy link
Contributor

Typescript Tests

    7 files    76 suites   5m 7s ⏱️
167 tests 163 ✔️   4 💤 0 ❌
582 runs  566 ✔️ 16 💤 0 ❌

Results for commit 7f48a16.

@jkaster jkaster requested a review from jhardy November 15, 2021 17:56
Copy link
Contributor

@josephaxisa josephaxisa left a comment

Choose a reason for hiding this comment

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

LGTM but I think it's worth adding a few test cases for ResponseExplorer

Comment on lines 129 to 130
const [activePathParams, setActivePathParams] = useState({})
const [activeQueryParams, setActiveQueryParams] = useState({})
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: given how tightly coupled path and query params are and that they are always updated together, consider merging these into one state object, activeRequestParams. Resultant state updates can then happen in one call.

@github-actions
Copy link
Contributor

APIX Tests

    1 files    77 suites   2m 53s ⏱️
322 tests 309 ✔️ 13 💤 0 ❌
332 runs  319 ✔️ 13 💤 0 ❌

Results for commit de55787.

@github-actions
Copy link
Contributor

Typescript Tests

    7 files    76 suites   5m 4s ⏱️
167 tests 163 ✔️   4 💤 0 ❌
582 runs  566 ✔️ 16 💤 0 ❌

Results for commit de55787.

@github-actions
Copy link
Contributor

APIX Tests

    1 files    77 suites   3m 42s ⏱️
322 tests 308 ✔️ 13 💤 1 ❌
332 runs  318 ✔️ 13 💤 1 ❌

For more details on these failures, see this check.

Results for commit 7753063.

@github-actions
Copy link
Contributor

Typescript Tests

    7 files    76 suites   4m 58s ⏱️
167 tests 163 ✔️   4 💤 0 ❌
582 runs  566 ✔️ 16 💤 0 ❌

Results for commit 7753063.

@jkaster jkaster marked this pull request as draft November 17, 2021 22:17
@jkaster
Copy link
Contributor Author

jkaster commented Nov 17, 2021

This just got a bit ugly when I was trying to achieve full URL derivation when used as an extension, so this is going to need a little bit of redesign, possibly as part of a RunIt adaptor interface.

@github-actions
Copy link
Contributor

APIX Tests

    1 files    77 suites   3m 27s ⏱️
324 tests 311 ✔️ 13 💤 0 ❌
334 runs  321 ✔️ 13 💤 0 ❌

Results for commit efef5e6.

@github-actions
Copy link
Contributor

Typescript Tests

    7 files    76 suites   4m 28s ⏱️
167 tests 163 ✔️   4 💤 0 ❌
582 runs  566 ✔️ 16 💤 0 ❌

Results for commit efef5e6.

@drstrangelooker drstrangelooker force-pushed the main branch 4 times, most recently from 9474788 to 5f9930c Compare February 4, 2022 02:33
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.

2 participants