Skip to content

Arch/qa env seeding tweaks#7430

Open
MGibson1 wants to merge 3 commits intomainfrom
arch/qa-env-seeding-tweaks
Open

Arch/qa env seeding tweaks#7430
MGibson1 wants to merge 3 commits intomainfrom
arch/qa-env-seeding-tweaks

Conversation

@MGibson1
Copy link
Copy Markdown
Member

@MGibson1 MGibson1 commented Apr 9, 2026

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-34880
https://bitwarden.atlassian.net/browse/PM-34881
https://bitwarden.atlassian.net/browse/PM-34886

📔 Objective

Various tweaks to smooth qa automation testing.

📸 Screenshots

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

Logo
Checkmarx One – Scan Summary & Details3582801a-f97b-4fef-b908-6e6a2030c1cf

Great job! No new security vulnerabilities introduced in this pull request

@MGibson1 MGibson1 force-pushed the arch/qa-env-seeding-tweaks branch from 7af0b81 to c19faae Compare April 9, 2026 22:32
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 9, 2026

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2026

Codecov Report

❌ Patch coverage is 19.04762% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.44%. Comparing base (4883d49) to head (c19faae).
⚠️ Report is 22 commits behind head on main.

Files with missing lines Patch % Lines
...lling/Services/Implementations/LicensingService.cs 20.00% 23 Missing and 5 partials ⚠️
...SharedWeb/Utilities/ServiceCollectionExtensions.cs 0.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7430      +/-   ##
==========================================
- Coverage   58.53%   58.44%   -0.09%     
==========================================
  Files        2069     2067       -2     
  Lines       91306    91016     -290     
  Branches     8128     8085      -43     
==========================================
- Hits        53443    53197     -246     
+ Misses      35954    35916      -38     
+ Partials     1909     1903       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MGibson1 MGibson1 marked this pull request as ready for review April 13, 2026 17:40
@MGibson1 MGibson1 requested a review from a team as a code owner April 13, 2026 17:40
@MGibson1 MGibson1 requested a review from kdenney April 13, 2026 17:40
@MGibson1 MGibson1 added the ai-review-vnext Request a Claude code review using the vNext workflow label Apr 13, 2026
Copy link
Copy Markdown
Contributor

@kdenney kdenney left a comment

Choose a reason for hiding this comment

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

Logic looks good! Only thing I'd ask is if you could add some unit tests to cover the new changes? We've had issues in the past with changes to the licensing logic causing regressions so I like to make sure we cover that area pretty thoroughly. Thanks!

{
ValidateIssuerSigningKey = true,
IssuerSigningKey = new X509SecurityKey(_certificate),
IssuerSigningKey = issuerKey,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
IssuerSigningKey = issuerKey,
IssuerSigningKeys =_verificationCertificates.Select((c) => new X509SecurityKey(c)),

This should also mean you don't need to do any merges since only one would be valid anyways. This just says both of those keys are valid.

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

Labels

ai-review-vnext Request a Claude code review using the vNext workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants