-
Notifications
You must be signed in to change notification settings - Fork 124
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
Generic_work_change_set_spec #2166
Conversation
@@ -362,7 +362,7 @@ | |||
|
|||
# rubocop:disable RSpec/ExampleLength | |||
it do | |||
is_expected.to eq ["created_at", "updated_at", "depositor", "title", | |||
is_expected.to include("created_at", "updated_at", "depositor", "title", |
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.
Can you explain the rationale for this change?
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.
Per Justin's point, just for fun I switched include
to contain_exactly
and get an error that import_url
is returned, but not expected, so does this reveal a bug?
1) GenericWorkChangeSet#fields should contain exactly "created_at", "updated_at", "depositor", "title", "date_uploaded", "date_modified", "admin_set_id", "state", "proxy_depositor", "on_behalf_of", "arkivo_checksum", "member_of_collection_ids", "member_ids", "thumbnail_id", "representative_id", "label", "relative_path", "resource_type", "creator", "contributor", "description", "keyword", "license", "rights_statement", "publisher", "date_created", "subject", "language", "identifier", "related_url", "source", and "based_near"
Failure/Error:
is_expected.to contain_exactly("created_at", "updated_at", "depositor", "title",
"date_uploaded", "date_modified", "admin_set_id",
"state", "proxy_depositor", "on_behalf_of",
"arkivo_checksum", "member_of_collection_ids",
"member_ids", "thumbnail_id", "representative_id",
"label", "relative_path", "resource_type", "creator",
"contributor", "description", "keyword", "license",
"rights_statement",
"publisher", "date_created", "subject", "language",
"identifier", "related_url", "source", "based_near")
expected collection contained: ["admin_set_id", "arkivo_checksum", "based_near", "contributor", "created_at", "creator", "date_creat...urce_type", "rights_statement", "source", "state", "subject", "thumbnail_id", "title", "updated_at"]
actual collection contained: ["admin_set_id", "arkivo_checksum", "based_near", "contributor", "created_at", "creator", "date_creat...urce_type", "rights_statement", "source", "state", "subject", "thumbnail_id", "title", "updated_at"]
the extra elements were: ["import_url"]
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.
test fails, so rather than re-order it seemed better to not require order (since order doesn't matter) ... just testing with contains_exactly
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.
My commit just fixes some whitespace offenses on rubocop, feel free to squash it into the original commit once the review is complete |
"rights_statement", | ||
"publisher", "date_created", "subject", "language", | ||
"identifier", "related_url", "source", "based_near"] | ||
is_expected.to include("created_at", "updated_at", "depositor", "title", |
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.
Is contain_exactly
what we want here then?
include ... order doesn't matter; added 'import_url' foo
13b309a
to
d1804f6
Compare
Looks like this needs a rebase. |
…2166) include ... order doesn't matter; added 'import_url' foo
Fixes one failure where result of eq is not in same order. Order doesn't matter here so use include.