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

Fix Search2ApiTest and Search2ApiVariationTest #5181

Closed
wants to merge 1 commit into from

Conversation

PenghaiZhang
Copy link
Contributor

@PenghaiZhang PenghaiZhang commented Feb 5, 2025

Checklist
Description of change

Update the test cases for calling search2 endpoint with MIME type filters. Previously, the test cases check if the number of Items is exactly 6. This was problematic since the number can be impacted by other tests. So the fix is to check if the number is greater than 0.

@PenghaiZhang
Copy link
Contributor Author

We need to merge GItLab to here again due to the springsource issue.

Copy link
Member

@edalex-ian edalex-ian left a comment

Choose a reason for hiding this comment

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

Gah! I hate to say this @PenghaiZhang (as it'd be great just to get this fixed), but unfortunately this change almost makes the test useless.

What I mean by this, is if you look at the first test validMimeTypeSearch - Search for a known MIME type. Now one would argue that this is attempting to do a search which should have different results due to the presence of the provided mime type filter.

But with this change, there's no test to see if the additional parameter had any impact - other than it didn't outright break the search.

I guess for the second test largeNumberOfParameterSearch we can get away with on the idea the testing is just making sure lots of parameters doesn't break the search API. But then, if it's also not validating what's returned is it really testing that it still works.....

What do you think?

@PenghaiZhang
Copy link
Contributor Author

Gah! I hate to say this @PenghaiZhang (as it'd be great just to get this fixed), but unfortunately this change almost makes the test useless.

What I mean by this, is if you look at the first test validMimeTypeSearch - Search for a known MIME type. Now one would argue that this is attempting to do a search which should have different results due to the presence of the provided mime type filter.

But with this change, there's no test to see if the additional parameter had any impact - other than it didn't outright break the search.

I guess for the second test largeNumberOfParameterSearch we can get away with on the idea the testing is just making sure lots of parameters doesn't break the search API. But then, if it's also not validating what's returned is it really testing that it still works.....

What do you think?

In my opinion, there are three options, and the best way is to have a separate testing institution. If you look into the test for facet search which has a separate insitution, it never has such problems.

The second option is to revert the change made in the TestNG config file.

For the third option, let me use this example to illustrate the steps:

  1. The test searches with a special query and the query must make sure the number of items is fixed;
  2. Search again with both the query and the MIME type filters.

@edalex-ian
Copy link
Member

This ended up be resolved in a GitLab MR.

@edalex-ian edalex-ian closed this Feb 14, 2025
@edalex-ian edalex-ian deleted the fix/Search2ApiTest branch February 14, 2025 04:48
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