Skip to content

Conversation

@futa-ikeda
Copy link
Contributor

@futa-ikeda futa-ikeda commented Jun 27, 2022

Purpose

  • Add/edit test for files page functionality

Summary of Changes

  • Add data-test selectors
  • Edit Registration Files test
  • Edit FileBrowser component test
  • Edit Node Files test

Screenshot(s)

  • NA

Side Effects

  • I can sleep better at night knowing we have test coverage for some of this

QA Notes

  • I've added data-test selectors for the bulk file actions: data-test-bulk-copy-trigger, data-test-bulk-move-trigger, and data-test-bulk-delete-trigger
  • I've added a data-test selector for the green "Add new" omni button: data-test-add-new-trigger

@futa-ikeda futa-ikeda changed the title Add file tests [WIP][ENG-3880] Add file tests Jun 27, 2022
@coveralls
Copy link

coveralls commented Jun 27, 2022

Pull Request Test Coverage Report for Build 2580064259

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.07%) to 70.52%

Files with Coverage Reduction New Missed Lines %
app/packages/files/osf-storage-file.ts 1 85.71%
Totals Coverage Status
Change from base Build 2556145898: 0.07%
Covered Lines: 5215
Relevant Lines: 7183

💛 - Coveralls

Copy link
Contributor

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

A couple of questions, one missing data-analytics-name, and an errant commented bit of code.

Comment on lines 6 to 7
data-test-bulk-delete-trigger
@type='destroy'
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we could use a data-analytics-name here.

import { setupMirage } from 'ember-cli-mirage/test-support';
import { t } from 'ember-intl/test-support';
import { TestContext } from 'ember-test-helpers';
// import { percySnapshot } from 'ember-percy';
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably don't need this comment (or we should add a snapshot, whichever is more appropriate)

@futa-ikeda futa-ikeda changed the title [WIP][ENG-3880] Add file tests [ENG-3880] Add file tests Jun 28, 2022
@chth0n1x
Copy link
Contributor

These look really great Futa. We approached testing a bit differently with the Node Model instance, but this may be better. I just ran checks again and all looks good. Well done!

chth0n1x
chth0n1x previously approved these changes Jun 28, 2022
brianjgeiger
brianjgeiger previously approved these changes Jun 28, 2022
Copy link
Contributor

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

Yay tests!

@futa-ikeda futa-ikeda merged commit 11ff263 into CenterForOpenScience:develop Jul 5, 2022
@futa-ikeda futa-ikeda deleted the add-file-tests branch July 5, 2022 19:38
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.

4 participants