System Test Overhaul spec/system/judges/new_spec.rb - #6531
Conversation
Signed-off-by: Audrea Cook <audrea@codewitch.dev>
Signed-off-by: Audrea Cook <audrea@codewitch.dev>
Signed-off-by: Audrea Cook <audrea@codewitch.dev>
Signed-off-by: Audrea Cook <audrea@codewitch.dev>
Signed-off-by: Audrea Cook <audrea@codewitch.dev>
Signed-off-by: Audrea Cook <audrea@codewitch.dev>
Signed-off-by: Audrea Cook <audrea@codewitch.dev>
AudTheCodeWitch
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the system test for creating judges to improve test reliability and coverage. The changes address race conditions by ensuring database updates are verified only after page loading completes, and enhance test isolation by using unique test data for each example.
- Replaced single test with comprehensive test suite covering active/inactive judges, edge cases, and validation scenarios
- Implemented helper method
submit_judge_formto reduce code duplication - Added test coverage for long names, special characters, blank names, and duplicate name validation
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let(:active_name) { Faker::Name.unique.name } | ||
| let(:inactive_name) { Faker::Name.unique.name } |
There was a problem hiding this comment.
Using Faker::Name.unique.name in let blocks can cause issues since let is memoized but Faker.unique resets between test runs. Consider using let! or generating names directly in the tests to ensure truly unique values for each test execution.
|
|
||
| # rubocop:disable RSpec/ExampleLength | ||
| it "creates a judge with special characters in the name", :aggregate_failures do | ||
| special_name = "#{Faker::Lorem.characters(number: 30, min_alpha: 10, min_numeric: 5)}!@#$%^&*()" |
There was a problem hiding this comment.
The special characters test uses a magic number (30) and hardcoded special characters. Consider extracting these to named constants or using a more descriptive approach to make the test intent clearer.
| end | ||
|
|
||
| it "does not allow duplicate judge names in the same organization", :aggregate_failures do | ||
| duplicate_name = Faker::Name.unique.name |
There was a problem hiding this comment.
Using Faker::Name.unique.name here may conflict with the unique names generated in the let blocks at the top of the spec, potentially causing Faker to run out of unique combinations. Consider using a simple string or resetting Faker.unique before this test.
| duplicate_name = Faker::Name.unique.name | |
| duplicate_name = "Duplicate Judge Name" |
What github issue is this PR for, if any?
Resolves #6321
What changed, and why?
spec/system/judges/new_spec.rbto ensure each test verifies database updates only after the page has finished loading post-submit, addressing race conditions and flakiness.How is this tested? (please write rspec and jest tests!) 💖💪
spec/system/judges/new_spec.rb.Feelings gif (optional)