-
Notifications
You must be signed in to change notification settings - Fork 22
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
Add initial tests for 'globus endpoint search' #1071
Conversation
This adds the necessary mocking framework pieces to have a simulated endpoint search response, and a test against that response.
For simplicity, just check that the value passed appears in the full URL.
"GCP_mapped_collection", | ||
"GCP_guest_collection", | ||
"GCSv5_endpoint", | ||
"GCSv5_mapped_collection", | ||
"GCSv5_guest_collection", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these values appear in an enum somewhere? If so, it will be worthwhile to use that enum directly (or perhaps to compose this list and then make assertions that it matches the enum).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They do not appear in an enum (IMO, "thank goodness", str enums are so clunky...).
They're in a Literal
type annotation.
Would you like me to write the "magic" to extract them from that annotation? I've done it before and it's pretty easy, but it adds a little bit of complexity to the test module.
- explode space split strings into sets and sanity check - refine internal helper params and fixture metadata - make test criteria for table output clearer - parse the query string for testing - also, test name normalization of the filter_entity_type param
This command has not had any functional test coverage, so these are the first tests for the command.
--filter-entity-type
was added in #1055, but it was not possible to test it at the time because we had no fixture data to work off of.To build initial data, I grabbed a live service search result and rewrote it thoroughly to remove my account and collection details.
@pjhinton-globus, the second commit here shows what I wanted you to be able to add.
When I realized that we'd need to setup new fixture data for endpoint search, I felt it was unfair to ask for this as part of an incremental addition to an existing, untested command.
I haven't discussed my decision to use
str.split()
to build out the data with anyone yet, which is probably the most contentious thing I've done here.