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

EP Merge: Wait for EP400 contentions #2700

Merged
merged 6 commits into from
Mar 5, 2024

Conversation

dfitchett
Copy link
Contributor

What was the problem?

Contentions are not available immediately upon receipt of a merge requests.

Associated tickets or Slack threads:

How does this fix it?1

Changes allow retrying to get the contentions every 2 seconds for up to a minute.

How to test this PR

  • pull code
  • Unit Test:
    • from /domain-ee/ee-ep-merge-app run pytest
  • Integration Tests:
    • run base platform
    • from /domain-ee/ee-ep-merge-app run pytest ./integration
  • End to End tests:
    • run this CI action to verify successful integration test
    • run end to end tests locally:
      • export BIP_CLAIM_URL=mock-bip-claims-api:20300
      • run bgs and bip services and mocks:
        • COMPOSE_PROFILES="bip,bgs" ./gradlew :app:dockerComposeUp
        • COMPOSE_PROFILES="bip,bgs" ./gradlew -p mocks :dockerComposeUp
      • ./gradlew :domain-ee:ee-ep-merge-app:endToEndTest

Footnotes

  1. Pull-Requests guidelines. If PR is significant, update Current Software State wiki page.

… max of 30 attempts. Updated unit/integration/end2end tests.
@dfitchett dfitchett requested review from a team as code owners March 1, 2024 20:54
Copy link
Contributor

github-actions bot commented Mar 1, 2024

Test Results

149 tests  ±0   149 ✅ ±0   47s ⏱️ -1s
 39 suites ±0     0 💤 ±0 
 39 files   ±0     0 ❌ ±0 

Results for commit f17dc5f. ± Comparison against base commit ac8b5d4.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Mar 1, 2024

JaCoCo Test Coverage

Overall Project 76.44%

There is no coverage information present for the Files changed

Copy link
Contributor

@agile-josiah agile-josiah left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nanotone nanotone left a comment

Choose a reason for hiding this comment

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

Good stuff! Just a few suggestions.

Copy link
Contributor

@nanotone nanotone left a comment

Choose a reason for hiding this comment

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

I noticed you left some occurrences of retry_rate; wanna rename them all while you're at it? Also open to keeping this as-is if you had a reason.

Anyway, approving this 👍

@dfitchett dfitchett merged commit 28c05b6 into develop Mar 5, 2024
1 check passed
@dfitchett dfitchett deleted the dfitchett/ep-merge/wait-for-contentions branch March 5, 2024 18:50
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