Maintenance: add comments to SecretsProvider integration tests

[dreamorosi]

Summary

In #1263 we have added integration tests for SecretsProvider, part of the upcoming Parameters utility.

We should dedicate some additional cycles to add comments/documentation to the tests to make them future proof and clearer.

Why is this needed?

To allow future maintainers and potential prospect contributors to more easily understand what's going on in the tests, as well as troubleshoot issues.

Which area does this relate to?

Tests, Parameters

Solution

See example of comments in the DynamoDBProvider here.

Acknowledgment

• This request meets Lambda Powertools Tenets
• Should this be considered in other Lambda Powertools languages? i.e. Python, Java

[am29d]

I can take this one, since I worked on the PR before.

[dreamorosi]

While at it, please also update the imports of the secretsProvider.class.test.functionCode.ts file to:

import {
  SecretsProvider,
} from '../../src/secrets';
import { SecretsGetOptionsInterface } from '../../src/types';

Currently they are pointing to the built/compiled version instead of the source.

And in tests 6 & 7, review the test key in the log emitted within the catch blocks, it should match the one in their respective try.

[dreamorosi]

Also, test number 7 we have:

await providerWithMiddleware.get(secretNamePlainChached);
await providerWithMiddleware.get(secretNamePlainChached, { forceFetch: true });

The counter here should be 2 (fetched twice because of using forceFetch), but in the test we are asserting as 1:

it('should retrieve single parameter twice without caching', async () => {
  const logs = invocationLogs[0].getFunctionLogs();
  const testLogFirst = InvocationLogs.parseFunctionLog(logs[6]);

  expect(testLogFirst).toStrictEqual({
    test: 'get-plain-force',
    value: 1 // This should be 2
  });
});

This is happening because we are fetching the same parameter from test 6, so the first time we fetch in test 7 we are still hitting cache, while in the second one we are actually finally fetching.

To fix this we have a few options:

• manually clear the cache before each test (providerWithMiddleware.clearCache();) - I'd go w/ this one
• fetch a different secret (so we avoid caching)

Apologies for not spotting these things during my review, I should have done better and I will try to in future ones.

[am29d]

No worries, I also missed this part completely.

Adding providerWithMiddleware.clearCache(); helps to clear the cache and the tests not works with value: 2.

[github-actions]

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.