Refactor: Project Samples Test #869
Conversation
This comment has been minimized.
This comment has been minimized.
ChrisHuynh333
force-pushed
the
refactor/project-samples-test
branch
3 times, most recently
from
713784f to
2376ede
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ChrisHuynh333
force-pushed
the
refactor/project-samples-test
branch
from
b0cefc6 to
973859e
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Code Metrics Report
Reported by octocov |
ChrisHuynh333
added
ready for review
| assert_no_selector 'form#deselect-all-form' | ||
| end | ||
|
|
||
| test 'User with role >= Maintainer sees sample table checkboxes' do |
There was a problem hiding this comment.
nitpick (blocking): Shouldn't a user with role ANALYST be able to see the checkboxes since they can launch workflow executions.
| assert_selector 'a', text: I18n.t('projects.samples.index.new_button'), count: 1 | ||
| within('#samples-table table tbody') do | ||
| # sample does not currently exist | ||
| assert_no_text 'New Name' |
There was a problem hiding this comment.
nitpick (blocking): This could pass even if a sample with New Name did exist in the project, since there is no assert to ensure that the samples table has been loaded.
| # verify sample exists in samples table | ||
| visit namespace_project_samples_url(@namespace, @project) | ||
| within('#samples-table table tbody') do | ||
| assert_text 'New Name' |
There was a problem hiding this comment.
nitpick (blocking): This could be potentially flakey as there is no assertion to ensure that the table has been loaded on the page.
| assert_selector 'h1', text: I18n.t(:'projects.samples.index.title'), count: 1 | ||
| # deleted sample row no longer exists | ||
| assert_no_selector "#samples-table table tbody tr[id='#{@sample1.id}']" |
There was a problem hiding this comment.
nitpick (blocking): Can you move this below the other assertion as that would ensure that the samples table has been loaded.
| assert_no_selector '#samples-table table tbody tr', text: @sample1.puid | ||
| assert_no_selector '#samples-table table tbody tr', text: @sample1.name | ||
| # sample no longer exists | ||
| assert_no_selector "tr[id='#{@sample1.id}']" |
There was a problem hiding this comment.
nitpick (blocking): Can you move this assertion after the limit_component assertion
| project2 = projects(:project2) | ||
| test 'transfer dialog sample listing' do | ||
| ### SETUP START ### | ||
| samples = @project.samples.pluck(:puid, :name) | ||
| visit namespace_project_samples_url(@namespace, @project) |
There was a problem hiding this comment.
nitpick (blocking): Can you add an assertion that the limit component is visible to ensure that the samples table has fully loaded to reduce flakes.
|
|
||
| test 'transfer dialog with plural description' do | ||
| ### SETUP START ### | ||
| visit namespace_project_samples_url(@namespace, @project) |
There was a problem hiding this comment.
nitpick (blocking): Same as comment above, please ensure table had fully loaded before proceeding to reduce changes of flakes.
| test 'should transfer a single sample' do | ||
| project2 = projects(:project2) | ||
| test 'transfer dialog with singular description' do | ||
| ### SETUP START ### | ||
| visit namespace_project_samples_url(@namespace, @project) |
There was a problem hiding this comment.
nitpick (blocking): Same as comment above.
| end | ||
| assert_text strip_tags(I18n.t(:'viral.pagy.limit_component.summary', from: 1, to: 20, count: 23, |
There was a problem hiding this comment.
nitpick (blocking): Can you assert for this above the samples assertions, that should reduce flakes
| login_as users(:user28) | ||
| namespace = groups(:group_hotel) | ||
| project = projects(:projectHotel) | ||
| visit namespace_project_samples_url(namespace, project) |
There was a problem hiding this comment.
nitpick (blocking): Can you assert for limit component before proceeding to reduce potential for flakes.
What does this PR do and why?
This PR refactors
system/projects/samples_test.rb.Upon revision, there were many aspects of the testing file that needed cleanup and/or fixing.
This is including but not limited to:
select2). These cases will have repetitive assertions and should be commented on.Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other pull requests.
How to set up and validate locally
PR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.