Skip to content

Conversation

@neumerance
Copy link
Collaborator

@neumerance neumerance commented May 29, 2025

Summary
This PR significantly increases the RSpec test coverage for the Amplify project, raising it from 47% to 80%.

@neumerance neumerance changed the base branch from master to develop May 29, 2025 00:33
@neumerance neumerance force-pushed the maintenance/increasing-test-coverage branch 5 times, most recently from cba402b to 304d3ee Compare May 29, 2025 03:04
@neumerance neumerance force-pushed the maintenance/increasing-test-coverage branch from 304d3ee to a1e77f6 Compare May 29, 2025 03:43
@neumerance neumerance changed the title [WIP] Maintenance/increasing test coverage Maintenance/increasing test coverage Jun 4, 2025
@rtrvrtg rtrvrtg requested review from ingnam and rtrvrtg June 5, 2025 04:05
end
resources :flags, only: [:index, :show, :create]
resources :transcript_speaker_edits, only: [:create]
resources :flags, only: [:index, :show, :create, :update, :destroy]
Copy link

Choose a reason for hiding this comment

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

Check if update and destory methods are being used at all

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

action exists. I want to keep this work black box approach as possible.


describe "GET #edit" do
it "assigns the transcription_convention and renders edit" do
get :edit, params: { institution_id: institution.id, id: transcription_convention.id }
Copy link

Choose a reason for hiding this comment

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

Make your tests DRYer wherever possible:
e.g.
let(:valid_attributes) { attributes_for(:transcription_convention) }
let(:invalid_attributes) { { convention_key: "" } }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I dont see any advantage of doing this

IMO expecitly defining expect(assigns(:transcription_conventions)).to include(transcription_convention) for each tests involve is easier to read and much manageable than drying it out

@neumerance neumerance force-pushed the maintenance/increasing-test-coverage branch from d6f8979 to 94fcfaa Compare June 17, 2025 05:30
@neumerance neumerance requested a review from ingnam June 17, 2025 05:40
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