refactor: Migrate Forwarder Tests to TypeScript #976

alexs-mparticle · 2025-02-10T14:52:21Z

Instructions

PR target branch should be against development
PR title name should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-title-check.yml
PR branch prefix should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-branch-check-name.yml

Summary

Migrates Forwarder Tests to TypeScript
Verifies imported interfaces and types to ensure they are properly implemented.

Testing Plan

Was this tested locally? If not, explain why.
Verify automated tests pass and the repo builds properly
Manually confirm Identity and Event Types are accessible via the mParticle object in the dev console.

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

Closes https://go.mparticle.com/work/SQDSDKS-5767

rmi22186

Looking good so far...

rmi22186 · 2025-02-11T13:25:19Z

src/mp-instance.ts

+    // this.EventType = EventType as unknown as valueof<typeof EventType>;
+    this.EventType = EventType;


Which one works here?

src/mp-instance.ts

rmi22186 · 2025-02-11T13:26:09Z

src/mparticle-instance-manager.ts

+    // this.EventType = EventType as unknown as valueof<typeof EventType>;
+    this.EventType = EventType;


ditto to comment above

rmi22186 · 2025-02-11T13:26:29Z

test/src/tests-integration-capture.ts

 } = Utils;

-const mParticle = window.mParticle;
+// interface MockMParticleForIntegrationCapture extends IMParticleInstanceManager {}


rmi22186 · 2025-02-11T13:27:06Z

test/src/tests-kit-blocking.ts

 import Types from '../../src/types';
 import { DataPlanVersion } from '@mparticle/data-planning-models';
 import fetchMock from 'fetch-mock/esm/client';
+// import { IMockForwarder } from './tests-forwarders';


not needed?

rmi22186 · 2025-02-11T13:35:40Z

test/src/tests-forwarders.ts

-        mParticle.logPageView();
-
-        Should(window.MockForwarder1.instance.receivedEvent).not.be.ok();
+        mParticle.logPageView('Page View');


This argument to logPageView is technically optional, so we should update that in the definition. When it is invoked with no arguments, it will automatically log event attributes with title and hostname, but when you pass an argument, it will set an attribute called screen_name

rmi22186 · 2025-02-11T13:38:43Z

test/src/tests-forwarders.ts


        mParticle.init(apiKey, window.mParticle.config);
-        await waitForCondition(hasIdentityCallInflightReturned);
+        await waitForCondition(() => window.mParticle.getInstance()?._Store?.identityCallInFlight === false);


Hm, why the change for this? Should we change the definition of hasIdentityCallInflightReturned to be this instead?

I think this a mistake on my part.

rmi22186 · 2025-02-11T13:43:34Z

test/src/tests-forwarders.ts

    });

-    it('should set only strings as integration attributes', async function() {
+    it('should set only strings as integration attributes', async () => {


i think renaming this to sanitize is better actually, otherwise it sounds like we may be converting to strings

Suggested change

it('should set only strings as integration attributes', async () => {

it('should sanitize any non-strings from integration attributes', async () => {

rmi22186 · 2025-02-11T13:47:05Z

test/src/tests-forwarders.ts

        // client calls reset
-        mParticle.reset();
+        // QUESTION: Should this actually take a parameter? it should probably be optional
+        mParticle.reset(null);


doesn't look like it based on the function signatures and how its used - in this case you are calling reset on the instance manager, which will automatically call the default instance and pass in that instance.

however, we should make a ticket to revisit reset in general and see how we can make it better. right now it's mostly used for tests, but i do believe customers have asked how they could perform some sort of similar functionality

rmi22186 · 2025-02-11T13:58:20Z

test/src/tests-forwarders.ts

+                // QUESTION: Where is this coming from?
+                // expect(mpInstance._Store.isUsingSideloadedKits).to.be.undefined;


This is likely a relic of a test when I was developing this. Putting my archaeology hat on...when i look at the store.ts, I see sideloadedKitsCount. So that probably suppllanted isUsingSideloadedKits and adds more info because if sideloadedKitsCount is greater than 0, that gives us the answer to "are they using side loaded kits", and also "how many are they using"

IIRC I added sideloadedKitsCount when we were trying to add metrics around side loaded kits. Not sure if that helps add context.

I say we probably should just remove this line since it's not a real method.

yea i agree with removing

rmi22186

one question otherwise looks great

test/src/tests-forwarders.ts

sonarqubecloud · 2025-02-17T15:52:42Z

Quality Gate failed

Failed conditions
39.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

mparticle-automation · 2025-03-15T02:55:10Z

🎉 This PR is included in version 2.34.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

alexs-mparticle force-pushed the test/SQDSDKS-5767-forwarder-tests-ts branch from b7b5e0c to f06f6e2 Compare February 10, 2025 14:53

alexs-mparticle added the refactor Changes to the structure of the code label Feb 10, 2025

rmi22186 requested changes Feb 11, 2025

View reviewed changes

alexs-mparticle marked this pull request as ready for review February 12, 2025 20:21

alexs-mparticle requested a review from rmi22186 February 12, 2025 20:21

rmi22186 requested changes Feb 13, 2025

View reviewed changes

test/src/tests-forwarders.ts Show resolved Hide resolved

rmi22186 approved these changes Feb 13, 2025

View reviewed changes

alexs-mparticle added the wait to merge label Feb 13, 2025

refactor: Migrate Forwarder Tests to TypeScript

351b0ff

alexs-mparticle force-pushed the test/SQDSDKS-5767-forwarder-tests-ts branch from 8d9f826 to 351b0ff Compare February 17, 2025 15:51

alexs-mparticle merged commit 40d4f1d into development Feb 28, 2025
21 of 29 checks passed

alexs-mparticle added a commit that referenced this pull request Mar 11, 2025

refactor: Migrate Forwarder Tests to TypeScript (#976)

1e80a36

mparticle-automation added the released label Mar 15, 2025

		// this.EventType = EventType as unknown as valueof<typeof EventType>;
		this.EventType = EventType;

	it('should set only strings as integration attributes', async () => {
	it('should sanitize any non-strings from integration attributes', async () => {

		// QUESTION: Where is this coming from?
		// expect(mpInstance._Store.isUsingSideloadedKits).to.be.undefined;

refactor: Migrate Forwarder Tests to TypeScript #976

refactor: Migrate Forwarder Tests to TypeScript #976

Uh oh!

Conversation

alexs-mparticle commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Instructions

Summary

Testing Plan

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

Uh oh!

rmi22186 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmi22186 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Feb 17, 2025

Quality Gate failed

Uh oh!

Uh oh!

mparticle-automation commented Mar 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alexs-mparticle commented Feb 10, 2025 •

edited

Loading