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

Add test to check that image viewer is rendered on the exhibits show page #3062

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

corylown
Copy link
Contributor

Prevent a future regression of the issue reported in #2992 and fixed in #3060

@@ -0,0 +1,22 @@
# frozen_string_literal: true

describe 'Exhibits show page', type: :feature do
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
describe 'Exhibits show page', type: :feature do
RSpec.describe 'Exhibits show page', type: :feature do

Avoid the need for monkeypatching.

Copy link
Member

@jcoyne jcoyne left a comment

Choose a reason for hiding this comment

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

It seems to me this is the same scenario as is set up in

visit spotlight.exhibit_solr_document_path(exhibit, 'dq287tq6352')
. Can they be merged?

@corylown corylown force-pushed the 2992-image-viewer branch from 864f936 to 3e995bc Compare July 19, 2024 15:24
@corylown
Copy link
Contributor Author

. Can they be merged?

Yes, specs are combined now. Thank you.

@@ -25,6 +25,11 @@
visit spotlight.search_exhibit_catalog_path(exhibit)
expect(page).to have_selector '.card-header', text: 'Item visibility'
end

it 'has an edit link' do
Copy link
Member

Choose a reason for hiding this comment

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

Feature tests are relatively slow (especially in the setup). Can we include the edit link assertion in the test above? I think the feature is "it draws the page" and there are several assertions that support "the page is drawn correctly".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two admin tests are hitting different routes so I didn't think it made sense to combine them. The test above is for search_exhibit_catalog_path and the Edit link test I added is for exhibit_solr_document_path

Copy link
Member

Choose a reason for hiding this comment

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

Oh, okay!

@jcoyne jcoyne merged commit 0ece91e into main Jul 19, 2024
8 checks passed
@jcoyne jcoyne deleted the 2992-image-viewer branch July 19, 2024 15:49
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