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

chore: Fixed aws-sdk-v3 bedrock tests (again) #2212

Merged
merged 1 commit into from
May 23, 2024

Conversation

jsumners-nr
Copy link
Contributor

@jsumners-nr jsumners-nr commented May 23, 2024

It seems like the passing versioned tests when I ran locally were a fluke. I managed to get a run locally that exhibits the CI failure (https://github.com/newrelic/node-newrelic/actions/runs/9211125804/job/25339758931), and that allowed me to inspect what is happening. It turns out that we have the following dependency tree for the actual agent:

❯ npm ls @smithy/smithy-client
newrelic@11.17.0 /Users/jsumners/Projects/team-repos/node-newrelic
├─┬ @aws-sdk/client-s3@3.582.0
│ ├─┬ @aws-sdk/client-sso-oidc@3.582.0
│ │ └── @smithy/smithy-client@3.0.1 deduped
│ ├─┬ @aws-sdk/client-sts@3.582.0
│ │ └── @smithy/smithy-client@3.0.1 deduped
│ ├─┬ @aws-sdk/core@3.582.0
│ │ └── @smithy/smithy-client@3.0.1 deduped
│ ├─┬ @aws-sdk/credential-provider-node@3.582.0
│ │ ├─┬ @aws-sdk/credential-provider-http@3.582.0
│ │ │ └── @smithy/smithy-client@3.0.1 deduped
│ │ └─┬ @aws-sdk/credential-provider-sso@3.582.0
│ │   └─┬ @aws-sdk/client-sso@3.582.0
│ │     └── @smithy/smithy-client@3.0.1 deduped
│ ├─┬ @aws-sdk/middleware-sdk-s3@3.582.0
│ │ └── @smithy/smithy-client@3.0.1 deduped
│ ├─┬ @smithy/core@2.0.1
│ │ └── @smithy/smithy-client@3.0.1 deduped
│ ├─┬ @smithy/middleware-retry@3.0.1
│ │ └── @smithy/smithy-client@3.0.1 deduped
│ ├── @smithy/smithy-client@3.0.1
│ ├─┬ @smithy/util-defaults-mode-browser@3.0.1
│ │ └── @smithy/smithy-client@3.0.1 deduped
│ └─┬ @smithy/util-defaults-mode-node@3.0.1
│   └── @smithy/smithy-client@3.0.1 deduped
└─┬ @aws-sdk/s3-request-presigner@3.582.0
  └── @smithy/smithy-client@3.0.1 deduped

And then we can have the tree in the test suite as:

❯ npm ls @smithy/smithy-client         
aws-sdk-v3-tests@0.0.0 /Users/jsumners/Projects/team-repos/node-newrelic/test/versioned/aws-sdk-v3
└─┬ @aws-sdk/client-bedrock-runtime@3.474.0 extraneous
  ├─┬ @aws-sdk/client-sso@3.474.0 extraneous
  │ └── @smithy/smithy-client@2.5.1 deduped
  ├─┬ @aws-sdk/client-sts@3.474.0 extraneous
  │ └── @smithy/smithy-client@2.5.1 deduped
  ├─┬ @aws-sdk/core@3.474.0 extraneous
  │ └── @smithy/smithy-client@2.5.1 deduped
  ├─┬ @aws-sdk/token-providers@3.470.0 extraneous
  │ └── @smithy/smithy-client@2.5.1 deduped
  ├─┬ @smithy/core@1.4.2 extraneous
  │ └── @smithy/smithy-client@2.5.1 deduped
  ├─┬ @smithy/middleware-retry@2.3.1 extraneous
  │ └── @smithy/smithy-client@2.5.1 deduped
  ├── @smithy/smithy-client@2.5.1 extraneous
  ├─┬ @smithy/util-defaults-mode-browser@2.2.1 extraneous
  │ └── @smithy/smithy-client@2.5.1 deduped
  └─┬ @smithy/util-defaults-mode-node@2.3.1 extraneous
    └── @smithy/smithy-client@2.5.1 deduped

Notice that the agent's tree has a different version of @smithy/smithy-client than the test suite tree. This is the crux of the problem. So we have updated the test suite to not need the package.json from @smithy/smithy-client at all. Instead, we look for the expected metric by its prefix and treat that as a good hit.

@jsumners-nr jsumners-nr force-pushed the fix-bedrock-tests-again branch from 0260808 to 8cef266 Compare May 23, 2024 17:20
@jsumners-nr jsumners-nr requested a review from bizob2828 May 23, 2024 17:47
@bizob2828 bizob2828 added the dev:tests Indicates only changes to tests label May 23, 2024
@jsumners-nr jsumners-nr merged commit 330cc4b into newrelic:main May 23, 2024
27 checks passed
@jsumners-nr jsumners-nr deleted the fix-bedrock-tests-again branch May 23, 2024 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev:tests Indicates only changes to tests
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants