Skip to content

chore: fix bedrock transport tests - #3735

Merged
akshaydeo merged 1 commit into
devfrom
05-25-chore_fix_bedrock_transport_tests
May 25, 2026
Merged

chore: fix bedrock transport tests#3735
akshaydeo merged 1 commit into
devfrom
05-25-chore_fix_bedrock_transport_tests

Conversation

@TejasGhatte

@TejasGhatte TejasGhatte commented May 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Replaces nil logger arguments with a noopLogger{} instance in Bedrock transport tests to ensure tests use a valid logger implementation rather than a nil value.

Changes

  • Replaced nil with noopLogger{} when calling NewBedrockProvider across all Bedrock transport tests to provide a proper no-op logger, avoiding potential nil pointer dereferences during test execution.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./core/providers/bedrock/...

All Bedrock transport tests should pass without nil pointer panics or logger-related errors.

Screenshots/Recordings

N/A

Breaking changes

  • Yes
  • No

Related issues

N/A

Security considerations

No security implications. This is a test-only change that swaps a nil logger for a no-op implementation.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 42fcfcfd-733d-448d-971d-b485b3c3c613

📥 Commits

Reviewing files that changed from the base of the PR and between 584816b and 6a4a944.

📒 Files selected for processing (1)
  • core/providers/bedrock/transport_test.go

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • Tests
    • Updated Bedrock transport configuration test infrastructure.

Note: This release contains internal test improvements with no user-facing changes to functionality.

Walkthrough

Update eight Bedrock transport configuration tests to initialize NewBedrockProvider with noopLogger{} instead of nil.

Changes

Bedrock Transport Test Logger Updates

Layer / File(s) Summary
Transport test logger initialization
core/providers/bedrock/transport_test.go
Eight transport test cases for HTTP/2 config, max connections, TLS configuration, and HTTP/2 enforcement now pass a concrete noopLogger{} instance instead of nil to NewBedrockProvider.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested reviewers

  • akshaydeo
  • danpiths

Poem

🐰 With logger's eyes now open bright,
The tests see through the transport light,
No nil shall pass—the noop's in place,
A logger true in every test's embrace!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: fix bedrock transport tests' directly and clearly summarizes the main change - fixing Bedrock transport tests by replacing nil loggers with noopLogger instances.
Description check ✅ Passed The PR description covers all critical sections of the template including Summary, Changes, Type of change, Affected areas, How to test, Breaking changes, and Security considerations with appropriate detail and completeness.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 05-25-chore_fix_bedrock_transport_tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Comment @coderabbitai help to get the list of available commands and usage tips.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@TejasGhatte
TejasGhatte marked this pull request as ready for review May 25, 2026 14:29
@coderabbitai
coderabbitai Bot requested review from akshaydeo and danpiths May 25, 2026 14:30
@greptile-apps

greptile-apps Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Test-only change that swaps nil for a well-defined no-op logger; no production code is touched.

Every changed call site now uses the noopLogger{} already defined in the same file, which fully satisfies the schemas.Logger interface. The fix is consistent with the rest of the test suite and carries no risk of behavioral change outside of tests.

No files require special attention.

Important Files Changed

Filename Overview
core/providers/bedrock/transport_test.go Replaces 8 nil logger arguments with noopLogger{} in Bedrock transport tests; noopLogger is already defined in the same file and fully implements schemas.Logger.

Reviews (1): Last reviewed commit: "chore: fix bedrock transport tests" | Re-trigger Greptile

akshaydeo commented May 25, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • May 25, 4:34 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 25, 4:35 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo merged commit 85aec7e into dev May 25, 2026
14 of 15 checks passed
@akshaydeo
akshaydeo deleted the 05-25-chore_fix_bedrock_transport_tests branch May 25, 2026 16:35
akshaydeo pushed a commit that referenced this pull request May 26, 2026
## Summary

Replaces `nil` logger arguments with a `noopLogger{}` instance in Bedrock transport tests to ensure tests use a valid logger implementation rather than a nil value.

## Changes

- Replaced `nil` with `noopLogger{}` when calling `NewBedrockProvider` across all Bedrock transport tests to provide a proper no-op logger, avoiding potential nil pointer dereferences during test execution.

## Type of change

- [ ] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [x] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./core/providers/bedrock/...
```

All Bedrock transport tests should pass without nil pointer panics or logger-related errors.

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

No security implications. This is a test-only change that swaps a nil logger for a no-op implementation.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
@akshaydeo akshaydeo mentioned this pull request May 27, 2026
18 tasks
akshaydeo added a commit that referenced this pull request May 27, 2026
## Summary

This PR releases Bifrost OSS `v1.5.5` and Enterprise `v1.4.4`, bumping all module pins from `v1.5.12`/`v1.3.12` to `v1.5.13`/`v1.3.13` across core, framework, and all plugins. It also hardens the Docker manifest shell scripts, expands CI egress allowlists, and updates documentation to reflect the new SCIM-based user provisioning feature.

## Changes

- **Module version bumps**: All `go.mod`/`go.sum` files updated from `core v1.5.12` → `v1.5.13`, `framework v1.3.12` → `v1.3.13`, and all plugin versions incremented accordingly (`compat`, `governance`, `jsonparser`, `logging`, `maxim`, `mocker`, `otel`, `prompts`, `semanticcache`, `telemetry`).
- **Docker manifest scripts**: Added `#!/usr/bin/env bash` shebang and `set -euo pipefail` to `create-docker-manifest.sh` and `create-docker-manifest-ubi9.sh`; quoted all variable expansions and switched `jq -r` to `jq -er` to fail on null digests.
- **CI egress allowlist**: Added `production.cloudfront.docker.com:443` to Docker-related job allowlists, and added `_https._tcp.dl.google.com:443` and `motd.ubuntu.com:443` to the Ubuntu package job allowlist.
- **Changelog files**: Cleared per-module `changelog.md` files (content moved into the new versioned docs). Added `docs/changelogs/v1.5.5.mdx` and `docs/changelogs/ent-v1.4.4.mdx` with full release notes, and registered both in `docs/docs.json`.
- **Documentation**: Replaced the SSO Integration link with a User Provisioning (SCIM) link in both `README.md` and `transports/README.md`.
- **Enterprise v1.4.4 highlights** (documented): Kafka and Google Cloud Pub/Sub observability sinks, chunked streaming with a 100 MB inter-node message ceiling, BigQuery custom labels via env vars using the new `ConfigMarshallerPlugin` interface, temporary access token expiry extensions, and a multi-node cluster integration harness.
- **OSS v1.5.5 highlights** (documented): Azure v1 API migration, env-var support for OTel/Prometheus configs, OTel extra-header forwarding and semantic-convention alignment, virtual key quota including provider configs, Responses API streaming in `jsonparser`, and a batch of Bedrock, Gemini, Anthropic, Azure, and logging plugin fixes.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [x] Documentation
- [x] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [x] Docs

## How to test

```sh
# Core/Transports
go version
go test ./...

# Verify Docker manifest scripts exit on error
bash -n .github/workflows/scripts/create-docker-manifest.sh
bash -n .github/workflows/scripts/create-docker-manifest-ubi9.sh
```

Validate that the new changelog pages (`changelogs/v1.5.5` and `changelogs/ent-v1.4.4`) render correctly in the docs site.

## Screenshots/Recordings

N/A

## Breaking changes

- [x] Yes
- [ ] No

The Azure provider no longer accepts `api_version` in `AzureKeyConfig` and has migrated to the `/openai/v1/{operation}` URL pattern. See the [v1.4.0 Migration Guide](https://docs.getbifrost.ai/enterprise/migration-guides/v1.4.0) for full details.

## Related issues

#3661, #3756, #3651, #3730, #3732, #3754, #3747, #3690, #3729, #3685, #3733, #3761, #3735, #3721, #3720, #3749, #3698, #3762, #3750, #3727, #3717, #3759, #3758, #3764, #3691, #3692, #3737, #3763

## Security considerations

- The `ConfigMarshallerPlugin` interface redacts secrets (OTel collector URLs, Prometheus push gateway credentials, BigQuery labels) at config storage time and rehydrates them at load time, preventing plaintext secret persistence.
- Docker manifest scripts now use `set -euo pipefail`, preventing silent failures that could result in malformed or missing image manifests being pushed.

## Checklist

- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
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.

3 participants