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

Improve e2e tests & fix sorting bug #1575

Merged
merged 13 commits into from
Aug 24, 2023

Conversation

louise-davies
Copy link
Member

@louise-davies louise-davies commented Aug 18, 2023

Description

  • Fix sorting bug where React Query considered two requests for columns sorted in a different order (e.g. name asc first, title desc second and title desc first, name asc second) the same request, and so would incorrectly get the data from cache rather than querying
  • Fix sorting bug in that the formatted columns in the admin download table couldn't be sorted
  • Fix filtering bug in download status table where only the first filter you applied was checked. Now, you need to satisfy all the applied filters to see the row
  • Fix filtering bug in download status table where invalid date ranges threw exceptions and crashed the app. Now, this no longer happens
  • Remove linting errors
  • Delete unneeded tests
  • Improve efficiency by consolidating tests into fewer, longer tests
    (see https://docs.cypress.io/guides/references/best-practices#Creating-Tiny-Tests-With-A-Single-Assertion)
    • This has significantly affected the length of the dataview tests - probably around a 33% reduction I would estimate
  • lint the cypress code in the CI job
  • Fixed some tests that just didn't pass locally against sg-preprod for various reasons - now all tests should pass both on CI and locally

Testing instructions

Add a set up instructions describing how the reviewer should test the code

  • Review code
  • Check Actions build
  • Review changes to test coverage
  • {more steps here}

- the order of the sort object is important, but react-query
hashes items in the query key and so differently ordered objects are considered equal
So JSON stringify the sort object when passed as a query key param
- Remove linting errors
- Delete unneeded tests
- Improve efficiency by consolidating tests into fewer, longer tests
(see https://docs.cypress.io/guides/references/best-practices#Creating-Tiny-Tests-With-A-Single-Assertion)
- lint the cypress code in the CI job
@louise-davies louise-davies changed the title Improve e2e tests Improve e2e tests & fix sorting bug Aug 18, 2023
@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Patch coverage: 84.90% and project coverage change: -0.08% ⚠️

Comparison is base (230b50c) 96.28% compared to head (0ff8da5) 96.20%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1575      +/-   ##
===========================================
- Coverage    96.28%   96.20%   -0.08%     
===========================================
  Files          161      161              
  Lines         6869     6885      +16     
  Branches      2129     2139      +10     
===========================================
+ Hits          6614     6624      +10     
- Misses         235      240       +5     
- Partials        20       21       +1     
Flag Coverage Δ
common 95.53% <100.00%> (ø)
dataview 96.85% <ø> (ø)
download 96.32% <72.41%> (-0.52%) ⬇️
search 96.50% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...c/downloadStatus/downloadStatusTable.component.tsx 94.63% <69.23%> (-3.89%) ⬇️
...es/datagateway-common/src/api/dataPublications.tsx 100.00% <100.00%> (ø)
packages/datagateway-common/src/api/datafiles.tsx 100.00% <100.00%> (ø)
packages/datagateway-common/src/api/datasets.tsx 100.00% <100.00%> (ø)
...ages/datagateway-common/src/api/facilityCycles.tsx 100.00% <100.00%> (ø)
...ackages/datagateway-common/src/api/instruments.tsx 100.00% <100.00%> (ø)
...ages/datagateway-common/src/api/investigations.tsx 100.00% <100.00%> (ø)
...nloadStatus/adminDownloadStatusTable.component.tsx 99.31% <100.00%> (+<0.01%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- multiple filters didn't work, it only ever checked the first filter
- Add tests for bugs I found
- Fix linting errors
- Improve test efficiency
- Lint cypress code in CI
- Ensure cypress code is linted in CI
- Fixed linting errors
- Small refactor for increased efficiency
@louise-davies louise-davies marked this pull request as ready for review August 23, 2023 13:32
Copy link
Contributor

@joshuadkitenge joshuadkitenge left a comment

Choose a reason for hiding this comment

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

All the bug described are fixed. code LGTM

@louise-davies louise-davies merged commit ec7d542 into develop Aug 24, 2023
9 checks passed
@louise-davies louise-davies deleted the bugfix/fix-cypress-linting-errors branch August 24, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants