From 2860ff4dd090d758d95eb69f1b50832dddd3c702 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Barc=C3=A9los?= Date: Fri, 4 Feb 2022 09:52:03 -0300 Subject: [PATCH] Add support for testing full summary (#860) --- packages/core/src/result-summary.ts | 23 +++++-- .../src/cypher-native-binders.js | 27 ++++++++ .../testkit-backend/src/request-handlers.js | 12 ++-- .../src/skipped-tests/common.js | 13 ++++ .../testkit-backend/src/summary-binder.js | 68 +++++++++++++++++++ 5 files changed, 129 insertions(+), 14 deletions(-) create mode 100644 packages/testkit-backend/src/summary-binder.js diff --git a/packages/core/src/result-summary.ts b/packages/core/src/result-summary.ts index 3daabebd2..09c3545df 100644 --- a/packages/core/src/result-summary.ts +++ b/packages/core/src/result-summary.ts @@ -333,6 +333,9 @@ class Stats { class QueryStatistics { private _stats: Stats private _systemUpdates: number + private _containsSystemUpdates?: boolean + private _containsUpdates?: boolean + /** * Structurize the statistics * @constructor @@ -350,7 +353,7 @@ class QueryStatistics { indexesAdded: 0, indexesRemoved: 0, constraintsAdded: 0, - constraintsRemoved: 0 + constraintsRemoved: 0, } this._systemUpdates = 0 Object.keys(statistics).forEach(index => { @@ -360,6 +363,10 @@ class QueryStatistics { this._stats[camelCaseIndex] = intValue(statistics[index]) } else if (camelCaseIndex === 'systemUpdates') { this._systemUpdates = intValue(statistics[index]) + } else if (camelCaseIndex === 'containsSystemUpdates') { + this._containsSystemUpdates = statistics[index] + } else if (camelCaseIndex === 'containsUpdates') { + this._containsUpdates = statistics[index] } }) @@ -371,11 +378,12 @@ class QueryStatistics { * @return {boolean} */ containsUpdates(): boolean { - return ( - Object.keys(this._stats).reduce((last, current) => { - return last + this._stats[current] - }, 0) > 0 - ) + return this._containsUpdates !== undefined ? + this._containsUpdates : ( + Object.keys(this._stats).reduce((last, current) => { + return last + this._stats[current] + }, 0) > 0 + ) } /** @@ -391,7 +399,8 @@ class QueryStatistics { * @returns {boolean} - If the system database get updated or not. */ containsSystemUpdates(): boolean { - return this._systemUpdates > 0 + return this._containsSystemUpdates !== undefined ? + this._containsSystemUpdates : this._systemUpdates > 0 } /** diff --git a/packages/testkit-backend/src/cypher-native-binders.js b/packages/testkit-backend/src/cypher-native-binders.js index 37f31227d..f7852e174 100644 --- a/packages/testkit-backend/src/cypher-native-binders.js +++ b/packages/testkit-backend/src/cypher-native-binders.js @@ -4,6 +4,33 @@ export function valueResponse (name, value) { return { name: name, data: { value: value } } } + +export function objectToCypher (obj) { + return objectMapper(obj, nativeToCypher) +} + +export function objectMemberBitIntToNumber (obj, recursive=false) { + return objectMapper(obj, val => { + if(typeof val === 'bigint') { + return Number(val) + } else if (recursive && typeof val === 'object') { + return objectMemberBitIntToNumber(val) + } else if (recursive && typeof val === 'array') { + return val.map(item => objectMemberBitIntToNumber(item, true)) + } + return val + }) +} + +function objectMapper(obj, mapper) { + if (obj === null || obj === undefined) { + return obj + } + return Object.keys(obj).reduce((acc, key) => { + return {...acc, [key]: mapper(obj[key])} + }, {}) +} + export function nativeToCypher (x) { if (x == null) { return valueResponse('CypherNull', null) diff --git a/packages/testkit-backend/src/request-handlers.js b/packages/testkit-backend/src/request-handlers.js index 1380661c0..fca0dec93 100644 --- a/packages/testkit-backend/src/request-handlers.js +++ b/packages/testkit-backend/src/request-handlers.js @@ -1,6 +1,7 @@ import neo4j from './neo4j' import ResultObserver from './result-observer.js' import { cypherToNative, nativeToCypher } from './cypher-native-binders.js' +import { nativeToTestkitSummary } from './summary-binder.js' import tls from 'tls' const SUPPORTED_TLS = (() => { @@ -182,18 +183,14 @@ export function ResultNext (context, data, wire) { } export function ResultConsume (context, data, wire) { + + const { resultId } = data const resultObserver = context.getResultObserver(resultId) resultObserver .completitionPromise() .then(summary => { - wire.writeResponse('Summary', { - ...summary, - serverInfo: { - agent: summary.server.agent, - protocolVersion: summary.server.protocolVersion.toFixed(1) - } - }) + wire.writeResponse('Summary', nativeToTestkitSummary(summary)) }) .catch(e => wire.writeError(e)) } @@ -336,6 +333,7 @@ export function GetFeatures (_context, _params, wire) { 'Temporary:ConnectionAcquisitionTimeout', 'Temporary:DriverMaxConnectionPoolSize', 'Temporary:FastFailingDiscovery', + 'Temporary:FullSummary', 'Temporary:ResultKeys', ...SUPPORTED_TLS ] diff --git a/packages/testkit-backend/src/skipped-tests/common.js b/packages/testkit-backend/src/skipped-tests/common.js index a01a6b48c..2b687e37a 100644 --- a/packages/testkit-backend/src/skipped-tests/common.js +++ b/packages/testkit-backend/src/skipped-tests/common.js @@ -27,6 +27,19 @@ const skippedTests = [ 'Not support by the JS driver', ifEquals('neo4j.sessionrun.TestSessionRun.test_partial_iteration') ), + skip( + 'Driver does not validate query type values in result summaries', + ifEquals('stub.summary.test_summary.TestSummary.test_invalid_query_type') + ), + skip( + 'ResultSummary.notifications defaults to empty array instead of return null/undefined', + ifEquals('stub.summary.test_summary.TestSummary.test_no_notifications'), + ifEquals('neo4j.test_summary.TestSummary.test_no_notification_info') + ), + skip( + 'ResultSummary.plan defaults to empty array instead of return null/undefined', + ifEquals('neo4j.test_summary.TestSummary.test_no_plan_info') + ), skip( 'The driver has no support domain_name_resolver', ifEndsWith('test_should_successfully_acquire_rt_when_router_ip_changes'), diff --git a/packages/testkit-backend/src/summary-binder.js b/packages/testkit-backend/src/summary-binder.js new file mode 100644 index 000000000..4dbab6ae7 --- /dev/null +++ b/packages/testkit-backend/src/summary-binder.js @@ -0,0 +1,68 @@ +import { objectToCypher, objectMemberBitIntToNumber } from './cypher-native-binders.js' + +function mapPlan(plan) { + return { + operatorType: plan.operatorType, + args: plan.arguments, + identifiers: plan.identifiers, + children: plan.children ? plan.children.map(mapPlan) : undefined + } +} + +function mapCounters(stats) { + return { + ...stats._stats, + systemUpdates: stats.systemUpdates(), + containsUpdates: stats.containsUpdates(), + containsSystemUpdates: stats.containsSystemUpdates() + } +} + +function mapProfile(profile, child=false) { + const mapChild = (child) => mapProfile(child, true) + const obj = { + args: objectMemberBitIntToNumber(profile.arguments), + dbHits: Number(profile.dbHits), + identifiers: profile.identifiers, + operatorType: profile.operatorType, + rows: Number(profile.rows), + children: profile.children ? profile.children.map(mapChild) : undefined + } + + if (child) { + return { + ...obj, + pageCacheHitRatio: profile.pageCacheHitRatio !== undefined ? Number(profile.pageCacheHitRatio) : undefined, + pageCacheHits: profile.pageCacheHits !== undefined ? Number(profile.pageCacheHits) : undefined, + pageCacheMisses: profile.pageCacheMisses !== undefined ? Number(profile.pageCacheMisses) : undefined, + time: profile.time !== undefined ? Number(profile.time) : undefined, + } + } + return obj +} + +function mapNotification(notification) { + return { + ...notification, + position: Object.keys(notification.position).length !== 0 ? notification.position : undefined, + } +} + +export function nativeToTestkitSummary (summary) { + return { + ...objectMemberBitIntToNumber(summary), + database: summary.database.name, + query: { + text: summary.query.text, + parameters: objectToCypher(summary.query.parameters) + }, + serverInfo: { + agent: summary.server.agent, + protocolVersion: summary.server.protocolVersion.toFixed(1) + }, + counters: mapCounters(summary.counters), + plan: mapPlan(summary.plan), + profile: mapProfile(summary.profile), + notifications: summary.notifications.map(mapNotification) + } +}