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(mirage): implement additional mock route handlers #1092

Merged
merged 6 commits into from
Aug 24, 2023

Conversation

tthvo
Copy link
Member

@tthvo tthvo commented Aug 22, 2023

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Related to #1039

Description of the change:

  • Improvement to Mirage api/v2.1/discovery route handler to ensure Realm group is shown.
    • Without these changes, realm group is not showed.
    • Also, creating a custom target will cause the other existing target to not be showed in graph view. List view works fine.
  • Fixed FormData handler. Mirage JS will fake parse the FormData (i.e. FormData object instead of string is returned when accessing body).
    • Previously, creating a recording or custom target will throw errors.
    • Original code on cryostat.github.io did this correctly.
    • Perhaps, some final commits in feat(dev): mock API server with miragejs #1039 accidentally added the JSON.parse that caused the issue.

Motivation for the change:

Overall mock API improvement and bug fixes.

@tthvo tthvo added fix feat New feature or request labels Aug 22, 2023
@tthvo tthvo requested review from andrewazores and maxcao13 August 22, 2023 04:12
@mergify mergify bot added the safe-to-test label Aug 22, 2023
@tthvo tthvo requested a review from aali309 August 22, 2023 04:16
@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1092-27765b013c57e72924484926a1bbbe15eedeedba sh smoketest.sh

@tthvo
Copy link
Member Author

tthvo commented Aug 22, 2023

image

Copy link
Member

@andrewazores andrewazores left a comment

Choose a reason for hiding this comment

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

Looks good, I will let @mwangggg and @aali309 have a look and ask some questions about what all this is before merging.

maxcao13
maxcao13 previously approved these changes Aug 22, 2023
Copy link
Member

@maxcao13 maxcao13 left a comment

Choose a reason for hiding this comment

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

LGTM, original bugs are fixed! :)

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1092-84877b89a07eb869278af97eae48f7ccd42f0947 sh smoketest.sh

@tthvo
Copy link
Member Author

tthvo commented Aug 22, 2023

Looks good, I will let @mwangggg and @aali309 have a look and ask some questions about what all this is before merging.

References

We use MirageJS to intercept network requests and mock API response: https://miragejs.com/docs/getting-started/introduction/

We also use mock-socket to mock the notification socket: https://www.npmjs.com/package/mock-socket.

How its build

There is a directory src/mirage that contains all codes for mocking. These are isolated from the rest of the application code. Webpack will include it as a chunk in web asset and ensures it is executed before actual application code.

https://webpack.js.org/concepts/entry-points/

How to run/test

To run a dev server with mirage, run yarn start:dev:preview.

To build production asset with mirage, run yarn build:preview:notests.

@maxcao13
Copy link
Member

References

We use MirageJS to intercept network requests and mock API response: https://miragejs.com/docs/getting-started/introduction/
...

Knowledge hub content :)

Copy link
Member

@maxcao13 maxcao13 left a comment

Choose a reason for hiding this comment

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

Actually there is still some functionality that is broken, notably: ARCHIVE recordings and STOP recordings (both part of a PATCH endpoint). Again because of JSON.parse.

@tthvo
Copy link
Member Author

tthvo commented Aug 22, 2023

Ahh yess! Missed that in the commits. Added now ^^

@tthvo
Copy link
Member Author

tthvo commented Aug 22, 2023

Tho, rules cannot be created because we need the matchExpression handler. I will add this.

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1092-d823e54935e993a65a0ebfa4da6ab863b5d0f90c sh smoketest.sh

@tthvo tthvo changed the title feat(mirage): improve discovery route response and fix FormData handler feat(mirage): implement additional mock route handlers Aug 22, 2023
@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1092-90d928aa130123f9e0f8eb4e66f45efe02d18869 sh smoketest.sh

@tthvo
Copy link
Member Author

tthvo commented Aug 22, 2023

Ready to review again! Added some missing route handlers to complete the rule functionalities.

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1092-3ab6fa26c57201260eb6f59e65d4af29d63a622c sh smoketest.sh

@tthvo tthvo requested a review from mwangggg August 22, 2023 22:22
andrewazores
andrewazores previously approved these changes Aug 23, 2023
@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-1092-81fd48806d35ee92b6f4ef38c56dd9b07d619173 sh smoketest.sh

@andrewazores andrewazores merged commit c185969 into cryostatio:main Aug 24, 2023
@tthvo tthvo deleted the mirage branch August 24, 2023 21:25
mergify bot pushed a commit that referenced this pull request Aug 31, 2023
(cherry picked from commit c185969)

# Conflicts:
#	src/mirage/factories.ts
#	src/mirage/index.ts
andrewazores pushed a commit that referenced this pull request Aug 31, 2023
andrewazores added a commit that referenced this pull request Aug 31, 2023
… (#1100)

* feat(mirage): implement additional mock route handlers (#1092)

(cherry picked from commit c185969)

* fixup

---------

Co-authored-by: Thuan Vo <thvo@redhat.com>
Co-authored-by: Andrew Azores <aazores@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants