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

[#1986] Add cypress tests for renderFilterHash #2017

Merged
merged 10 commits into from
Jan 23, 2024

Conversation

nseah21
Copy link
Contributor

@nseah21 nseah21 commented Jul 6, 2023

Fixes #1986

Add cypress tests for renderFilterHash

There were a few instances of URL parameters being incorrectly
loaded due to bugs in the code. 

These bugs were not able to be detected early due to a lack of 
tests for the renderFilterHash method.

Let's add tests to check that the URL parameters are loaded
correctly after the application reloads.

Other information

NA

@nseah21 nseah21 changed the title Add cypress tests for renderFilterHash [#1986] Add cypress tests for renderFilterHash Jul 6, 2023
@ckcherry23 ckcherry23 requested a review from a team July 9, 2023 15:12
Copy link
Member

@ckcherry23 ckcherry23 left a comment

Choose a reason for hiding this comment

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

Thanks for taking this up @nseah21 and sorry for the late review.
I think the test cases you have added now seem pretty comprehensive.

However, we may want to wait until issue #2016 is fixed before adding a test case for that scenario within this PR. This same issue is also apparent during the test case 'checked file types: url params should persist after change and reload' as the yml option gets unchecked by itself after reload.

@github-actions
Copy link
Contributor

Hi,
We are going to mark this PR as stale because it has been inactive for the past 30 days.
If no further activity occurs within the following 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 7 days and leave a comment to remove the stale label.
Do let us know if you are stuck so that we can help you!'

Copy link
Member

@ckcherry23 ckcherry23 left a comment

Choose a reason for hiding this comment

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

Since it is taking a while for issue #2016 to be resolved, I will approve this PR and update that issue regarding these Cypress tests. Thanks @nseah21 for your work!

@ckcherry23 ckcherry23 requested a review from a team October 3, 2023 23:04
@ckcherry23 ckcherry23 requested a review from vvidday October 16, 2023 11:17
@ckcherry23 ckcherry23 requested review from vvidday and a team and removed request for a team and vvidday November 19, 2023 17:57
@ckcherry23 ckcherry23 requested a review from chan-j-d December 11, 2023 05:27
Copy link
Contributor

Hi,
We are going to mark this PR as stale because it has been inactive for the past 30 days.
If no further activity occurs within the following 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 7 days and leave a comment to remove the stale label.
Do let us know if you are stuck so that we can help you!'

Copy link
Member

@MarcusTXK MarcusTXK left a comment

Choose a reason for hiding this comment

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

LGTM, merging this in so that more tests can be added in #2086.

@MarcusTXK MarcusTXK merged commit e02ab12 into reposense:master Jan 23, 2024
10 checks passed
Copy link
Contributor

The following links are for previewing this pull request:

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

Successfully merging this pull request may close these issues.

Add cypress test for renderFilterHash()
4 participants