Skip to content

Commit 4d19ec4

Browse files
authored
Fix integration tests (#220)
* MCP: fix typos of "vulnerabilities" instead of "problems" * Fix davis-copilot integration test * fix and expand on auth integration tests - Original test send-email.integration.test.ts failed because createDtHttpClient now throws the authorization exception, not sendEmail. - Moved authentication test into its own dynatrace-clients.integration.test.ts - Create authorization test for send-email * Fix integration tests for find-monitored-entity - Use findMonitoredEntitiesByName (the old findMonitoredEntityByName no longer exists) - Result is no longer a string, but a DqlExecutionResult - Fix when no entities provided (throw exception) - Removed test for empty string (instead have test for empty list of entities)
1 parent 0e95498 commit 4d19ec4

File tree

6 files changed

+103
-33
lines changed

6 files changed

+103
-33
lines changed

integration-tests/davis-copilot-explain-dql.integration.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,17 @@ describe('DQL Explanation Integration Tests', () => {
102102

103103
expect(response.status === 'SUCCESSFUL' || response.status === 'SUCCESSFUL_WITH_WARNINGS').toBeTruthy();
104104

105+
// Can say any of:
106+
// - "...calculate the total number of logs"
107+
// - "...calculate the total number of log entries"
108+
// - "...count the number of logs"
109+
// - "...count the number of log entries"
110+
//
111+
// Can also fail intermittently with:
112+
// - "I'm sorry, but I can't generate a response for you right now due to unusually high
113+
// demand. Please try again in a few minutes."
105114
expect(response.summary.toLowerCase()).toContain('group logs by');
106-
expect(response.summary.toLowerCase()).toContain('calculate the total number of logs');
115+
expect(response.summary.toLowerCase()).toMatch(/calculate the total number of log|count the number of log/);
107116
// The explanation should be reasonably detailed
108117
expect(response.explanation.length).toBeGreaterThan(50);
109118
});
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* Integration test for dynatrace client functionality
3+
*
4+
* Verifies authentication behaviour by making actual API calls
5+
* to the Dynatrace environment.
6+
*
7+
* The error tests use deliberately incorrect authentication credentials.
8+
*
9+
* Other integration tests will use dynatrace-clients, so the happy-path is
10+
* implicitly tested via other tests.
11+
*/
12+
13+
import { config } from 'dotenv';
14+
import { createDtHttpClient } from '../src/authentication/dynatrace-clients';
15+
import { getDynatraceEnv, DynatraceEnv } from '../src/getDynatraceEnv';
16+
17+
// Load environment variables
18+
config();
19+
20+
const API_RATE_LIMIT_DELAY = 100; // Delay in milliseconds to avoid hitting API rate limits
21+
22+
const scopesBase = [
23+
'app-engine:apps:run', // needed for environmentInformationClient
24+
'app-engine:functions:run', // needed for environmentInformationClient
25+
];
26+
27+
describe('Dynatrace Clients Integration Tests', () => {
28+
let dynatraceEnv: DynatraceEnv;
29+
30+
// Setup that runs once before all tests
31+
beforeAll(async () => {
32+
try {
33+
dynatraceEnv = getDynatraceEnv();
34+
console.log(`Testing against environment: ${dynatraceEnv.dtEnvironment}`);
35+
} catch (err) {
36+
throw new Error(`Environment configuration error: ${(err as Error).message}`);
37+
}
38+
});
39+
40+
afterEach(async () => {
41+
// Add delay to avoid hitting API rate limits
42+
await new Promise((resolve) => setTimeout(resolve, API_RATE_LIMIT_DELAY));
43+
});
44+
45+
describe('Error Handling', () => {
46+
it('should handle authentication errors gracefully', async () => {
47+
// Create client with invalid credentials
48+
await expect(
49+
createDtHttpClient(
50+
dynatraceEnv.dtEnvironment,
51+
scopesBase,
52+
'invalid-client-id',
53+
'invalid-client-secret',
54+
undefined,
55+
),
56+
).rejects.toThrow(`Failed to retrieve OAuth token`);
57+
}, 30000);
58+
});
59+
});

integration-tests/find-monitored-entity-by-name.integration.test.ts

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@
77

88
import { config } from 'dotenv';
99
import { createDtHttpClient } from '../src/authentication/dynatrace-clients';
10-
import { findMonitoredEntityByName } from '../src/capabilities/find-monitored-entity-by-name';
10+
import { findMonitoredEntitiesByName } from '../src/capabilities/find-monitored-entity-by-name';
1111
import { getDynatraceEnv, DynatraceEnv } from '../src/getDynatraceEnv';
12+
import { getEntityTypeFromId } from '../src/utils/dynatrace-entity-types';
1213

1314
// Load environment variables
1415
config();
@@ -60,27 +61,25 @@ describe('Find Monitored Entity by Name Integration Tests', () => {
6061

6162
// Search for an entity name that is very unlikely to exist
6263
const searchTerm = 'this-entity-definitely-does-not-exist-12345';
64+
const extendedSearch = false;
6365

64-
const response = await findMonitoredEntityByName(dtClient, searchTerm);
66+
const response = await findMonitoredEntitiesByName(dtClient, [searchTerm], extendedSearch);
6567

6668
expect(response).toBeDefined();
67-
expect(typeof response).toBe('string');
68-
expect(response).toBe('No monitored entity found with the specified name.');
69+
expect(response?.records).toBeDefined();
70+
expect(response?.records?.length).toEqual(0);
6971
}, 30_000); // Increased timeout for API calls
7072

71-
test('should handle search with empty string', async () => {
73+
test('should handle search with empty list', async () => {
7274
const dtClient = await createHttpClient();
7375

7476
// Test with empty string
75-
const searchTerm = '';
77+
const searchTerms = [] as string[];
78+
const extendedSearch = false;
7679

77-
const response = await findMonitoredEntityByName(dtClient, searchTerm);
78-
79-
expect(response).toBeDefined();
80-
expect(typeof response).toBe('string');
81-
82-
// Should handle gracefully - likely will return many results or handle empty search
83-
expect(response).toContain('You need to provide an entity name to search for');
80+
await expect(findMonitoredEntitiesByName(dtClient, searchTerms, extendedSearch)).rejects.toThrow(
81+
/No entity names supplied to search for/,
82+
);
8483
});
8584

8685
test('should return properly formatted response when entities are found', async () => {
@@ -89,20 +88,19 @@ describe('Find Monitored Entity by Name Integration Tests', () => {
8988
// Search for a pattern that is likely to find at least one entity
9089
// "host" is common in most Dynatrace environments
9190
const searchTerm = 'host';
91+
const extendedSearch = false;
9292

93-
const response = await findMonitoredEntityByName(dtClient, searchTerm);
93+
const response = await findMonitoredEntitiesByName(dtClient, [searchTerm], extendedSearch);
9494

95+
// Assert, based on the DqlExecutionResult
9596
expect(response).toBeDefined();
96-
expect(typeof response).toBe('string');
97-
98-
// If entities are found, check the format
99-
if (response.includes('The following monitored entities were found:')) {
100-
// Each line should follow the expected format
101-
const lines = response.split('\n').filter((line) => line.startsWith('- Entity'));
102-
103-
lines.forEach((line) => {
104-
expect(line).toMatch(/^- Entity '.*' of type '.* has entity id '.*'$/);
97+
if (response?.records && response.records.length > 0) {
98+
response.records.forEach((entity) => {
99+
expect(entity?.id).toBeDefined();
100+
expect(getEntityTypeFromId(String(entity?.id))).toBeDefined();
105101
});
102+
} else {
103+
// Nothing to assert; environment for testing has no entities found.
106104
}
107105
});
108106
});

integration-tests/send-email.integration.test.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -322,25 +322,25 @@ All recipients should receive this message according to their designation.`,
322322
});
323323

324324
describe('Error Handling', () => {
325-
it('should handle authentication errors gracefully', async () => {
325+
it('should handle authorization errors gracefully', async () => {
326326
// Create client with invalid credentials
327327
const dtClient = await createDtHttpClient(
328328
dynatraceEnv.dtEnvironment,
329-
scopesBase.concat(scopesEmail),
330-
'invalid-client-id',
331-
'invalid-client-secret',
332-
undefined,
329+
scopesBase.concat([]), // no scopesEmail
330+
dynatraceEnv.oauthClientId,
331+
dynatraceEnv.oauthClientSecret,
332+
dynatraceEnv.dtPlatformToken,
333333
);
334334

335335
const emailRequest: EmailRequest = {
336336
toRecipients: { emailAddresses: [TEST_EMAIL_TO] },
337-
subject: '[Integration Test] Authentication Error Test',
337+
subject: '[Integration Test] Authorization Error Test',
338338
body: {
339-
body: 'This should fail due to invalid credentials.',
339+
body: 'This should fail due to insufficient permissions.',
340340
},
341341
};
342342

343-
await expect(sendEmail(dtClient, emailRequest)).rejects.toThrow();
343+
await expect(sendEmail(dtClient, emailRequest)).rejects.toThrow(`Forbidden`);
344344
}, 30000);
345345

346346
it('should handle invalid email addresses appropriately', async () => {

src/capabilities/find-monitored-entity-by-name.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ import {
1313
* @returns DQL Statement for searching all entity types
1414
*/
1515
export const generateDqlSearchEntityCommand = (entityNames: string[], extendedSearch: boolean): string => {
16+
if (entityNames == undefined || entityNames.length == 0) {
17+
throw new Error(`No entity names supplied to search for`);
18+
}
19+
1620
// If extendedSearch is true, use all entity types, otherwise use only basic ones
1721
const fetchDqlCommands = (extendedSearch ? DYNATRACE_ENTITY_TYPES_ALL : DYNATRACE_ENTITY_TYPES_BASICS).map(
1822
(entityType, index) => {

src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ const main = async () => {
365365
if (!result || result.length === 0) {
366366
return 'No vulnerabilities found in the last 30 days';
367367
}
368-
let resp = `Found ${result.length} problems in the last 30 days! Displaying the top ${maxVulnerabilitiesToDisplay} problems:\n`;
368+
let resp = `Found ${result.length} vulnerabilities in the last 30 days! Displaying the top ${maxVulnerabilitiesToDisplay} vulnerabilities:\n`;
369369
result.slice(0, maxVulnerabilitiesToDisplay).forEach((vulnerability) => {
370370
resp += `\n* ${vulnerability}`;
371371
});

0 commit comments

Comments
 (0)