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

FS-52: Testing keyword search #71

Open
wants to merge 6 commits into
base: 3.x
Choose a base branch
from

Conversation

russom-woldezghi
Copy link
Contributor

@russom-woldezghi russom-woldezghi commented Mar 19, 2020

Ticket

FS-52: Testing Keyword Search

Description

  • Adds text keyword search (for autocomplete disabled) test coverage
  • Test search using the word "Terrier", expects 4 result items and 1 pager page generated.

Testing instruction

  • Pull down code
  • Remove node_modules directory
  • Run yarn install && yarn start
  • Run yarn cypress to start Cypress electron binary. This will open another app to run test.
  • Click on "Run all specs", this will open a Chrome to run all the steps.
  • Verify tests are running and all tests pass.

Screen Shot 2020-03-19 at 5 14 30 PM

Open question

  • I had to add an extra space character in the text field, because it was for some reason cutting off the first "T" of "Terrier" not sure if that is intended here but wanted to flag that.

@russom-woldezghi russom-woldezghi added the Work in-Progress Do not merge PR, currently being worked on. label Mar 19, 2020
@russom-woldezghi russom-woldezghi added Ready for Review and removed Work in-Progress Do not merge PR, currently being worked on. labels Mar 19, 2020

describe('Keyword search', () => {
const keyword = 'Terrier';
const resultCount = 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

@russom-woldezghi I retested this yesterday after destroying my vagrant and provisioning it again. I now get 2 results consistently on all sites including this one, so I think 2 might be correct. Can you verify with the current demo environment?

Copy link
Contributor

Choose a reason for hiding this comment

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

I get 5.

Screen Shot 2020-05-06 at 9 59 22 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

My result matches this

@biz123
Copy link
Contributor

biz123 commented May 6, 2020

In app tests I'm getting a failure for in the check for .fs-search-accordion__group-item

Screen Shot 2020-05-06 at 6 26 04 AM

@biz123 biz123 added bug Something isn't working and removed Ready for Review labels May 6, 2020
@biz123
Copy link
Contributor

biz123 commented May 6, 2020

I should add that when I run the behat tests within the federated search demo I get test fails there as well, so I may still need to fix something on that end.

@biz123 biz123 removed the bug Something isn't working label May 6, 2020
@biz123
Copy link
Contributor

biz123 commented May 6, 2020

@russom-woldezghi I am going to test again after resolving the issues where Behat tests were failing.

@biz123
Copy link
Contributor

biz123 commented May 6, 2020

@russom-woldezghi after troubleshooting my demo environment with @agentrickard I now have 5 results for terrier, so I've gone from too few (2 before) to too many (test expects 4). I'm still also getting a fail on the test for .fs-search-accordion__group-item.

@biz123
Copy link
Contributor

biz123 commented May 6, 2020

Screen Shot 2020-05-06 at 12 32 42 PM

@biz123 biz123 added the bug Something isn't working label May 6, 2020
@russom-woldezghi
Copy link
Contributor Author

The issue introduced was that I was missing a site index and was resulting in 4 not 5 results for the word "Terrier".

I wasn't able to reproduce the error with .fs-search-accordion__group-item class missing.

I will look into the hook registry issue Ken called out during scrum, just to make sure the error with .fs-search-accordion__group-item is not related.

@russom-woldezghi russom-woldezghi added Work in-Progress Do not merge PR, currently being worked on. and removed bug Something isn't working labels May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Work in-Progress Do not merge PR, currently being worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants