Skip to content

Commit

Permalink
fix: Remove API start()'s features param (#1009)
Browse files Browse the repository at this point in the history
  • Loading branch information
@cwli24
cwli24 authored May 2, 2024
1 parent 3a41fcc commit 38a502b
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 112 deletions.
2 changes: 1 addition & 1 deletion src/features/utils/instrument-base.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class InstrumentBase extends FeatureBase {
/** if the feature requires opt-in (!auto-start), it will get registered once the api has been called */
if (this.auto) registerDrain(agentIdentifier, featureName)
else {
this.ee.on(`${this.featureName}-opt-in`, single(() => {
this.ee.on('manual-start-all', single(() => {
// register the feature to drain only once the API has been called, it will drain when importAggregator finishes for all the features
// called by the api in that cycle
registerDrain(this.agentIdentifier, this.featureName)
Expand Down
16 changes: 3 additions & 13 deletions src/loaders/api/api.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,20 +117,10 @@ export function setAPI (agentIdentifier, forceDrain, runSoftNavOverSpa = false)
return appendJsAttribute('application.version', value, 'setApplicationVersion', false)
}

apiInterface.start = (features) => {
apiInterface.start = () => {
try {
const smTag = !features ? 'undefined' : 'defined'
handle(SUPPORTABILITY_METRIC_CHANNEL, [`API/start/${smTag}/called`], undefined, FEATURE_NAMES.metrics, instanceEE)
const featNames = Object.values(FEATURE_NAMES)
if (features === undefined) features = featNames
else {
features = Array.isArray(features) && features.length ? features : [features]
if (features.some(f => !featNames.includes(f))) return warn(`Invalid feature name supplied. Acceptable feature names are: ${featNames}`)
if (!features.includes(FEATURE_NAMES.pageViewEvent)) features.push(FEATURE_NAMES.pageViewEvent)
}
features.forEach(feature => {
instanceEE.emit(`${feature}-opt-in`)
})
handle(SUPPORTABILITY_METRIC_CHANNEL, ['API/start/called'], undefined, FEATURE_NAMES.metrics, instanceEE)
instanceEE.emit('manual-start-all')
} catch (err) {
warn('An unexpected issue occurred', err)
}
Expand Down
51 changes: 8 additions & 43 deletions tests/components/api.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,66 +468,31 @@ describe('setAPI', () => {
await new Promise(process.nextTick)
})

test('should create SM event emitter event for calls to API when features are undefined', () => {
test('should create SM event emitter event for calls to API', () => {
apiInterface.start()

expect(handleModule.handle).toHaveBeenCalledTimes(1)
expect(handleModule.handle).toHaveBeenCalledWith(
SUPPORTABILITY_METRIC_CHANNEL,
['API/start/undefined/called'],
['API/start/called'],
undefined,
FEATURE_NAMES.metrics,
instanceEE
)
})

test('should create SM event emitter event for calls to API when features are undefined', () => {
const features = [faker.string.uuid()]
apiInterface.start(features)

expect(handleModule.handle).toHaveBeenCalledTimes(1)
expect(handleModule.handle).toHaveBeenCalledWith(
SUPPORTABILITY_METRIC_CHANNEL,
['API/start/defined/called'],
undefined,
FEATURE_NAMES.metrics,
instanceEE
)
})

test('should emit event emitter events for all features when input is undefined', () => {
test('should emit event to start all features (if not auto)', () => {
apiInterface.start()

Object.values(FEATURE_NAMES).forEach(featureName => {
expect(instanceEE.emit).toHaveBeenCalledWith(`${featureName}-opt-in`)
})
})

test('should emit event emitter events for all features when input is set', () => {
apiInterface.start(Object.values(FEATURE_NAMES))

Object.values(FEATURE_NAMES).forEach(featureName => {
expect(instanceEE.emit).toHaveBeenCalledWith(`${featureName}-opt-in`)
})
expect(instanceEE.emit).toHaveBeenCalledWith('manual-start-all')
})

test('should return early and warn for invalid feature names', () => {
test('should emit start even if some arg is passed', () => {
const badFeatureName = faker.string.uuid()
apiInterface.start(badFeatureName)

Object.values(FEATURE_NAMES).forEach(featureName => {
expect(instanceEE.emit).not.toHaveBeenCalledWith(`${featureName}-opt-in`)
})
expect(instanceEE.emit).not.toHaveBeenCalledWith(`${badFeatureName}-opt-in`)
expect(console.warn).toHaveBeenCalledTimes(1)
expect(console.warn).toHaveBeenCalledWith(expect.stringContaining('Invalid feature name supplied'))
})

test('should always include page view event feature', () => {
apiInterface.start(['spa'])

expect(instanceEE.emit).toHaveBeenCalledWith('page_view_event-opt-in')
expect(instanceEE.emit).toHaveBeenCalledWith('spa-opt-in')
expect(instanceEE.emit).toHaveBeenCalledWith('manual-start-all')
expect(instanceEE.emit).not.toHaveBeenCalledWith(badFeatureName)
expect(console.warn).not.toHaveBeenCalled()
})
})

Expand Down
69 changes: 14 additions & 55 deletions tests/specs/api.e2e.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,55 +310,7 @@ describe('newrelic api', () => {
}
}

it('should not start features when provided feature name is invalid', async () => {
const results = await Promise.all([
browser.testHandle.expectRum(10000, true),
browser.testHandle.expectTimings(10000, true),
browser.testHandle.expectAjaxEvents(10000, true),
browser.testHandle.expectErrors(10000, true),
browser.testHandle.expectMetrics(10000, true),
browser.testHandle.expectIns(10000, true),
browser.testHandle.expectResources(10000, true),
browser.testHandle.expectInteractionEvents(10000, true),
browser.url(await browser.testHandle.assetURL('instrumented-manual.html'), config)
.then(() => browser.pause(1000))
.then(() => browser.execute(function () {
newrelic.start('INVALID')
setTimeout(function () {
window.location.reload()
}, 1000)
}))
.then(() => undefined)
])

expect(results).toEqual(new Array(9).fill(undefined))
})

it('should not start features when provided feature name is invalid type', async () => {
const results = await Promise.all([
browser.testHandle.expectRum(10000, true),
browser.testHandle.expectTimings(10000, true),
browser.testHandle.expectAjaxEvents(10000, true),
browser.testHandle.expectErrors(10000, true),
browser.testHandle.expectMetrics(10000, true),
browser.testHandle.expectIns(10000, true),
browser.testHandle.expectResources(10000, true),
browser.testHandle.expectInteractionEvents(10000, true),
browser.url(await browser.testHandle.assetURL('instrumented-manual.html'), config)
.then(() => browser.pause(1000))
.then(() => browser.execute(function () {
newrelic.start(1)
setTimeout(function () {
window.location.reload()
}, 1000)
}))
.then(() => undefined)
])

expect(results).toEqual(new Array(9).fill(undefined))
})

it('should start all features when passed undefined', async () => {
it('should start all features', async () => {
const initialLoad = await Promise.all([
browser.testHandle.expectRum(10000, true),
browser.url(await browser.testHandle.assetURL('instrumented-manual.html'), config)
Expand Down Expand Up @@ -394,28 +346,35 @@ describe('newrelic api', () => {
checkSpa(results[7].request)
})

it('should force start PVE when another feature is started', async () => {
it('starts everything if the auto features do not include PVE, and nothing should have started', async () => {
const initialLoad = await Promise.all([
browser.testHandle.expectRum(10000, true),
browser.url(await browser.testHandle.assetURL('instrumented-manual.html'), config)
.then(() => undefined)
browser.testHandle.expectErrors(10000, true),
browser.url(await browser.testHandle.assetURL('instrumented-manual.html', {
init: {
...config.init,
jserrors: {
autoStart: true
}
}
})).then(() => undefined)
])

expect(initialLoad).toEqual(new Array(2).fill(undefined))
expect(initialLoad).toEqual(new Array(3).fill(undefined))

const results = await Promise.all([
browser.testHandle.expectRum(10000),
browser.testHandle.expectErrors(10000),
browser.execute(function () {
newrelic.start('jserrors')
newrelic.start()
})
])

checkRum(results[0].request)
checkJsErrors(results[1].request, { messages: ['test'] })
})

it('should start the partial list of features', async () => {
it('starts the rest of the features if the auto features include PVE, and those should have started', async () => {
const results = await Promise.all([
browser.testHandle.expectRum(),
browser.testHandle.expectTimings(),
Expand Down

0 comments on commit 38a502b

Please sign in to comment.