Skip to content

Maintenance/increasing test coverage#148

Open
neumerance wants to merge 4 commits into
developfrom
maintenance/increasing-test-coverage
Open

Maintenance/increasing test coverage#148
neumerance wants to merge 4 commits into
developfrom
maintenance/increasing-test-coverage

Conversation

@neumerance

@neumerance neumerance commented May 29, 2025

Copy link
Copy Markdown
Collaborator

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
Comment thread config/routes.rb
end
resources :flags, only: [:index, :show, :create]
resources :transcript_speaker_edits, only: [:create]
resources :flags, only: [:index, :show, :create, :update, :destroy]

Copy link
Copy Markdown

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
Copy Markdown
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.

Comment thread spec/controllers/admin/themes_controller_spec.rb
Comment thread spec/controllers/admin/users_controller_spec.rb Outdated

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
Copy Markdown

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
Copy Markdown
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