Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate specs for status update jobs #19463

Closed
wants to merge 10 commits into from

Conversation

kayline
Copy link
Contributor

@kayline kayline commented Nov 14, 2024

Summary

  • This work is behind a feature toggle (flipper): YES/NO
  • Refactor only: Move identical tests to shared examples
  • Decision Reviews, yes

Related issue(s)

department-of-veterans-affairs/va.gov-team#96645

Testing done

  • New code is covered by unit tests
    -The same setup and behavior tests were duplicated for each of the three subclasses that implements the base job. Now there is a single shared examples file that each spec references
  • Break the job, ensure tests fail as expected

What areas of the site does it impact?

(Describe what parts of the site are impacted andifcode touched other areas)

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

Copy link

github-actions bot commented Nov 14, 2024

1 Error
🚫 This PR changes 1173 LoC (not counting whitespace/newlines).

In order to ensure each PR receives the proper attention it deserves, those exceeding
500 will not be reviewed, nor will they be allowed to merge. Please break this PR up into
smaller ones.

If you have reason to believe that this PR should be granted an exception, please see the
Submitting pull requests for approval - FAQ.

File Summary

Files

  • spec/sidekiq/decision_review/hlr_status_updater_job_spec.rb (+3/-131)

  • spec/sidekiq/decision_review/nod_status_updater_job_spec.rb (+4/-287)

  • spec/sidekiq/decision_review/sc_status_updater_job_spec.rb (+5/-426)

  • spec/sidekiq/decision_review/shared_examples_for_status_updater_jobs.rb (+317/-0)

    Note: We exclude files matching the following when considering PR size:

    *.csv, *.json, *.tsv, *.txt, *.md, Gemfile.lock, app/swagger, modules/mobile/docs, spec/fixtures/, spec/support/vcr_cassettes/, modules/mobile/spec/support/vcr_cassettes/, db/seeds, modules/vaos/app/docs, modules/meb_api/app/docs, modules/appeals_api/app/swagger/, *.bru, *.pdf
    

Big PRs are difficult to review, often become stale, and cause delays.

Generated by 🚫 Danger

@va-vfs-bot va-vfs-bot temporarily deployed to DR-status-update-shared-examples/main/main November 14, 2024 18:31 Inactive
@kayline kayline force-pushed the DR-status-update-shared-examples branch from e4f9659 to 5faec8e Compare November 15, 2024 18:01
@va-vfs-bot va-vfs-bot temporarily deployed to DR-status-update-shared-examples/main/main November 15, 2024 18:20 Inactive
@kayline kayline force-pushed the DR-status-update-shared-examples branch from 5b8da4a to d599afd Compare November 15, 2024 18:30
@va-vfs-bot va-vfs-bot temporarily deployed to DR-status-update-shared-examples/main/main November 15, 2024 18:36 Inactive
@kayline kayline force-pushed the DR-status-update-shared-examples branch from d599afd to 0880f9b Compare November 18, 2024 17:57
@va-vfs-bot va-vfs-bot temporarily deployed to DR-status-update-shared-examples/main/main November 18, 2024 18:34 Inactive
- add rescus to base class
- add tests to each subclass
- needs fix to decision review service to make method signatures match
- was :uuid, now :guid
- to match get_notice_of_disagreement_upload for easier code sharing and shared testing
@kayline kayline force-pushed the DR-status-update-shared-examples branch from 0880f9b to 418e840 Compare November 18, 2024 19:10
@va-vfs-bot va-vfs-bot temporarily deployed to DR-status-update-shared-examples/main/main November 18, 2024 19:23 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to DR-status-update-shared-examples/main/main November 18, 2024 20:28 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to DR-status-update-shared-examples/main/main November 18, 2024 21:22 Inactive
-fix conflicts in status updater job specs
@kayline
Copy link
Contributor Author

kayline commented Nov 21, 2024

Closing in favor of four smaller PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants