fix(telemetry): e2e suite singleton reset + relaxed flush-on-close assert

CI on the previous commit showed two e2e failures driven by state that
leaks across e2e test files, not by real defects:

1. `should initialize telemetry when telemetryEnabled is true` —
   `featureFlagCacheSpy.called` was false because a prior e2e suite had
   already created a TelemetryClient for the test host. Subsequent
   `getOrCreateClient` calls hit the existing holder and skip the
   constructor (and the FFCache.getOrCreateContext call inside it).
   Fixed by `before` + `beforeEach` `__resetInstanceForTests()` so this
   suite always sees a fresh provider regardless of execution order.
   That also clears the "second context, different auth" F4 warn that
   was firing on every test in this suite.

2. `should cleanup telemetry on close` — `MetricsAggregator.flush()`
   isn't called by the close-drain when `pendingMetrics` is empty
   (the test connects + closes without ever `openSession`-ing, so no
   events are queued). Replaced the strict `flushSpy.called` assert
   with `aggregatorCloseSpy.called` — `MetricsAggregator.close()` is
   the cleanup surface that always runs, regardless of buffer state.

Also: change the outer `describe` from `function ()` to arrow form
(eslint `prefer-arrow-callback`); only `before`/`it` need the
function-form for `this`.

Co-authored-by: Isaac
Signed-off-by: samikshya-chand_data <samikshya.chand@databricks.com>

Author: samikshya-db

tests/e2e/telemetry/telemetry-integration.test.ts

@@ -71,18 +71,30 @@ function loadConfigOrSkip(suite: Mocha.Suite): TestConfig | null {
   return cfg as TestConfig;
 }
 
-describe('Telemetry Integration', function () {
+describe('Telemetry Integration', () => {
   let config: TestConfig | null = null;
 
+  // The e2e test runner executes many suites before this one; any earlier
+  // DBSQLClient connect leaves a TelemetryClient in the process-wide singleton
+  // for the test host. Reset before our first test so the FF cache and
+  // refcount spies in this suite observe a clean lineage.
   before(function () {
     config = loadConfigOrSkip(this.test!.parent!);
     if (!config) {
       this.skip();
+      return;
     }
+    TelemetryClientProvider.__resetInstanceForTests();
   });
 
   // Reset the process-wide singleton between tests so refcount + cached
-  // feature flags from one test don't leak into the next.
+  // feature flags from one test don't leak into the next. Combined with the
+  // `before` reset above, every test sees a fresh provider regardless of
+  // what other e2e suites did first.
+  beforeEach(() => {
+    TelemetryClientProvider.__resetInstanceForTests();
+  });
+
   afterEach(() => {
     TelemetryClientProvider.__resetInstanceForTests();
     sinon.restore();
@@ -222,7 +234,7 @@ describe('Telemetry Integration', function () {
       const client = new DBSQLClient();
 
       const releaseClientSpy = sinon.spy(TelemetryClientProvider.prototype, 'releaseClient');
-      const flushSpy = sinon.spy(MetricsAggregator.prototype, 'flush');
+      const aggregatorCloseSpy = sinon.spy(MetricsAggregator.prototype, 'close');
 
       try {
         await client.connect({
@@ -234,15 +246,19 @@ describe('Telemetry Integration', function () {
 
         await client.close();
 
-        // All three must happen on a clean close: releaseClient is the
-        // refcount surface; flush is the drain. Previously a disjunction
-        // (`||`) over multiple spies meant any single one firing satisfied
-        // the test — a regression that broke ONE of them would still pass.
+        // releaseClient is the refcount surface; MetricsAggregator.close is
+        // the cleanup the last refcount holder triggers. Both must fire on a
+        // clean close. We do NOT assert flush() was called because this test
+        // never `openSession`s, so the pending-metrics buffer is empty and
+        // the close-drain pattern legitimately skips the final flush. The
+        // previous disjunction (`releaseClient || flush || releaseContext`)
+        // over multiple spies meant a regression breaking one would still
+        // pass — these explicit asserts catch that.
         expect(releaseClientSpy.called, 'releaseClient should be called on close').to.be.true;
-        expect(flushSpy.called, 'aggregator flush should run on close').to.be.true;
+        expect(aggregatorCloseSpy.called, 'MetricsAggregator.close should run on close').to.be.true;
       } finally {
         releaseClientSpy.restore();
-        flushSpy.restore();
+        aggregatorCloseSpy.restore();
       }
     });
   });