fix!: recreate mock on reset() (#76)

* fix: unify the behavior for direct behavior functions and onAnyCommand calls

* fix: create new mock instance on reset() call

This prevents mock behaviors breaking subsequent behaviors
between the reset() calls due to Sinon.JS bug #1572.

* fix!: remove resetBehavior() method

The resetBehavior() method, calling Sinon.JS stub.resetBehavior(),
cannot be relied upon because of Sinon.JS bug #1572.

src/awsClientStub.ts

@@ -1,5 +1,6 @@
 import {Client, Command, MetadataBearer} from '@aws-sdk/types';
 import {match, SinonSpyCall, SinonStub} from 'sinon';
+import {mockClient} from './mockClient';
 
 export type AwsClientBehavior<TClient extends Client<any, any, any>> =
     TClient extends Client<infer TInput, infer TOutput, any> ? Behavior<TInput, TOutput, TOutput> : never;
@@ -44,22 +45,24 @@ export class AwsStub<TInput extends object, TOutput extends MetadataBearer> impl
      */
     public send: SinonStub<[AwsCommand<TInput, TOutput>], Promise<TOutput>>;
 
-    private readonly anyCommandBehavior: CommandBehavior<TInput, TOutput, TOutput>;
-
-    constructor(send: SinonStub<[AwsCommand<TInput, TOutput>], Promise<TOutput>>) {
+    constructor(
+        private client: Client<TInput, TOutput, any>,
+        send: SinonStub<[AwsCommand<TInput, TOutput>], Promise<TOutput>>,
+    ) {
         this.send = send;
-        this.anyCommandBehavior = new CommandBehavior(this, send);
     }
 
-    /** Resets stub's history and behavior. */
+    /**
+     * Resets stub. It will replace the stub with a new one, with clean history and behavior.
+     */
     reset(): AwsStub<TInput, TOutput> {
-        this.send.reset();
-        return this;
-    }
-
-    /** Resets stub's behavior. */
-    resetBehavior(): AwsStub<TInput, TOutput> {
-        this.send.resetBehavior();
+        /* sinon.stub.reset() does not remove the fakes which in some conditions can break subsequent stubs,
+         * so instead of calling send.reset(), we recreate the stub.
+         * See: https://github.com/sinonjs/sinon/issues/1572
+         * We are only affected by the broken reset() behavior of this bug, since we always use matchers.
+         */
+        const newStub = mockClient(this.client);
+        this.send = newStub.send;
         return this;
     }
 
@@ -131,8 +134,6 @@ export class AwsStub<TInput extends object, TOutput extends MetadataBearer> impl
      * Allows specifying the behavior for any Command with given input (parameters).
      *
      * If the input is not specified, the given behavior will be used for any Command with any input.
-     * This is no different from using {@link resolves}, {@link rejects}, etc. directly,
-     * but can be used for readability.
      * @param input Command payload to match
      * @param strict Should the payload match strictly (default false, will match if all defined payload properties match)
      */
@@ -149,27 +150,33 @@ export class AwsStub<TInput extends object, TOutput extends MetadataBearer> impl
 
     /**
      * Sets a successful response that will be returned from any `Client#send()` invocation.
+     *
+     * Same as `mock.onAnyCommand().resolves()`.
      * @param response Content to be returned
      */
     resolves(response: CommandResponse<TOutput>): AwsStub<TInput, TOutput> {
-        return this.anyCommandBehavior.resolves(response);
+        return this.onAnyCommand().resolves(response);
     }
 
     /**
      * Sets a failure response that will be returned from any `Client#send()` invocation.
      * The response will always be an `Error` instance.
+     *
+     * Same as `mock.onAnyCommand().rejects()`.
      * @param error Error text, Error instance or Error parameters to be returned
      */
     rejects(error?: string | Error | AwsError): AwsStub<TInput, TOutput> {
-        return this.anyCommandBehavior.rejects(error);
+        return this.onAnyCommand().rejects(error);
     }
 
     /**
      * Sets a function that will be called on any `Client#send()` invocation.
+     *
+     * Same as `mock.onAnyCommand().callsFake()`.
      * @param fn Function taking Command input and returning result
      */
     callsFake(fn: (input: any) => any): AwsStub<TInput, TOutput> {
-        return this.anyCommandBehavior.callsFake(fn);
+        return this.onAnyCommand().callsFake(fn);
     }
 
 }

src/mockClient.ts

@@ -21,7 +21,7 @@ export const mockClient = <TInput extends object, TOutput extends MetadataBearer
 
     const sendStub = stub(instance, 'send') as SinonStub<[Command<TInput, any, TOutput, any, any>], Promise<TOutput>>;
 
-    return new AwsStub<TInput, TOutput>(sendStub);
+    return new AwsStub<TInput, TOutput>(instance, sendStub);
 };
 
 type ClassType<T> = {

test/fixtures.ts

@@ -1,6 +1,7 @@
 import {PublishCommand} from '@aws-sdk/client-sns';
 
 export const topicArn = 'arn:aws:sns:us-east-1:111111111111:MyTopic';
+export const topicArn2 = 'arn:aws:sns:us-east-1:111111111111:MyOtherTopic';
 
 export const publishCmd1 = new PublishCommand({
     TopicArn: topicArn,
@@ -10,6 +11,10 @@ export const publishCmd2 = new PublishCommand({
     TopicArn: topicArn,
     Message: 'second mock message',
 });
+export const publishCmd3 = new PublishCommand({
+    TopicArn: topicArn2,
+    Message: 'third mock message',
+});
 
 export const uuid1 = '12345678-1111-2222-3333-111122223333';
 export const uuid2 = '12345678-4444-5555-6666-111122223333';

test/mockClient.regression.test.ts

@@ -0,0 +1,53 @@
+import {AwsClientStub, mockClient} from '../src';
+import {PublishCommand, SNSClient} from '@aws-sdk/client-sns';
+import {publishCmd1, publishCmd3, topicArn} from './fixtures';
+
+let snsMock: AwsClientStub<SNSClient>;
+
+beforeEach(() => {
+    snsMock = mockClient(SNSClient);
+});
+
+afterEach(() => {
+    snsMock.restore();
+});
+
+/**
+ * Sinon.JS has a bug with a sinon.match breaking subsequent mocks in some scenarios,
+ * including leaking the mock behaviors between the stub.reset() calls.
+ * See: https://github.com/m-radzikowski/aws-sdk-client-mock/issues/67 and https://github.com/sinonjs/sinon/issues/1572
+ */
+describe('issue 67 - breaking subsequent mocks', () => {
+    const sns = new SNSClient({});
+
+    /**
+     * This corresponds to the pattern with mock.reset() and mock.onAnyCommand().rejects()
+     * being called in the beforeEach() block and then individual mock behaviors being set in test functions.
+     */
+    test('resetting mock does not break subsequent mocks', async () => {
+        snsMock.onAnyCommand().rejects('any command error');
+        snsMock.on(PublishCommand).rejects('publish error');
+
+        snsMock.reset();
+        snsMock.onAnyCommand().rejects('any command error');
+
+        const publish = sns.send(publishCmd1);
+
+        await expect(publish).rejects.toThrow('any command error');
+    });
+
+    /**
+     * Make sure the main behavior described in the Sinon.JS bug, with match.any breaking subsequent stubs,
+     * does not happen.
+     */
+    test('onAnyCommand does not brake other mocks', async () => {
+        snsMock.onAnyCommand().rejects('any command error');
+        snsMock.onAnyCommand({TopicArn: topicArn}).rejects('any command first topic error');
+
+        const publish1 = sns.send(publishCmd1);
+        const publish2 = sns.send(publishCmd3);
+
+        await expect(publish1).rejects.toThrow('any command first topic error');
+        await expect(publish2).rejects.toThrow('any command error');
+    });
+});

test/mockClient.test.ts

@@ -21,22 +21,6 @@ describe('setting up the mock', () => {
         expect(publish).toBeUndefined();
     });
 
-    it('resets mock behavior', async () => {
-        snsMock.resolves({
-            MessageId: uuid1,
-        });
-
-        const sns = new SNSClient({});
-        const publish1 = await sns.send(publishCmd1);
-
-        snsMock.resetBehavior();
-
-        const publish2 = await sns.send(publishCmd2);
-
-        expect(publish1.MessageId).toBe(uuid1);
-        expect(publish2).toBeUndefined();
-    });
-
     it('resets mock behavior on mock reset', async () => {
         snsMock.resolves({
             MessageId: uuid1,