fix: delete env var after test if it was not set

[olavloite]

Delete the environment variable SPANNER_EMULATOR_HOST after a test instead of setting it to undefined when it was not defined in the first place. Setting the environment variable to undefined will convert it to the string 'undefined' and could affect other test cases, as they would see the environment variable as having been set.

[codecov]

Codecov Report

Merging #774 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #774   +/-   ##
=======================================
  Coverage   74.58%   74.58%           
=======================================
  Files          43       43           
  Lines       20220    20220           
  Branches      546      546           
=======================================
  Hits        15082    15082           
  Misses       5133     5133           
  Partials        5        5

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de67b75...4317924. Read the comment docs.

[AVaksman]

LGTM!

proposing wrap these 5 tests in a describe and utilize beforeEach() and afterEach() to handle SPANNER_EMULATOR_HOST env var, then we can omit try ... finally block in each test.

    describe('SPANNER_EMULATOR_HOST', () => {
      let currentEmulator: string | undefined;

      beforeEach(() => (currentEmulator = process.env.SPANNER_EMULATOR_HOST));

      afterEach(() => {
        if (currentEmulator) {
          process.env.SPANNER_EMULATOR_HOST = currentEmulator;
        } else {
          delete process.env.SPANNER_EMULATOR_HOST;
        }
      });

      it('should parse emulator host without port correctly', () => {
        const EMULATOR_HOST = 'somehost.local';
        process.env.SPANNER_EMULATOR_HOST = `${EMULATOR_HOST}`;

        const emulator = Spanner.getSpannerEmulatorHost();

        assert.deepStrictEqual(emulator, {endpoint: EMULATOR_HOST});
      });
    });

[olavloite]

LGTM!

proposing wrap these 5 tests in a describe and utilize beforeEach() and afterEach() to handle SPANNER_EMULATOR_HOST env var, then we can omit try ... finally block in each test.

    describe('SPANNER_EMULATOR_HOST', () => {
      let currentEmulator: string | undefined;

      beforeEach(() => (currentEmulator = process.env.SPANNER_EMULATOR_HOST));

      afterEach(() => {
        if (currentEmulator) {
          process.env.SPANNER_EMULATOR_HOST = currentEmulator;
        } else {
          delete process.env.SPANNER_EMULATOR_HOST;
        }
      });

      it('should parse emulator host without port correctly', () => {
        const EMULATOR_HOST = 'somehost.local';
        process.env.SPANNER_EMULATOR_HOST = `${EMULATOR_HOST}`;

        const emulator = Spanner.getSpannerEmulatorHost();

        assert.deepStrictEqual(emulator, {endpoint: EMULATOR_HOST});
      });
    });

Done.
Thanks for the good tip. That is definitely a lot better.

[hengfengli]

LGTM.