Cherry pick release 2019.6.1 into master (microsoft#6519)

* Fix experiments hash parsing (microsoft#6492) * Simplify & fix crypto.createHash * Add tests * Fix python test in news folder (microsoft#6398) * Fix broken python tests in news * Revert changes * Release cherry pick (microsoft#6515) * Enable experiment for always displaying the test explorer (microsoft#6330) * Add logging * Point release - Updated package.json and CHANGELOG (microsoft#6516)

Author: kimadeline

CHANGELOG.md

@@ -56,6 +56,12 @@ And finally thanks to the [Python](https://www.python.org/) development team and
 community for creating a fantastic programming language and community to be a
 part of!
 
+## 2019.6.1 (9 July 2019)
+
+### Fixes
+
+1. Fixes to A/B testing.
+ ([#6400](https://github.com/microsoft/vscode-python/issues/6400))
 
 ## 2019.6.0 (25 June 2019)
 

src/client/common/crypto.ts

@@ -4,17 +4,25 @@
 
 // tslint:disable: no-any
 
-import { createHash, HexBase64Latin1Encoding } from 'crypto';
+import { createHash } from 'crypto';
 import { injectable } from 'inversify';
+import { traceError } from './logger';
 import { ICryptoUtils, IHashFormat } from './types';
 
 /**
  * Implements tools related to cryptography
  */
 @injectable()
 export class CryptoUtils implements ICryptoUtils {
- public createHash<E extends keyof IHashFormat>(data: string, encoding: HexBase64Latin1Encoding, hashFormat: E): IHashFormat[E] {
- const hash = createHash('sha512').update(data).digest(encoding);
- return hashFormat === 'number' ? parseInt(hash, undefined) : hash as any;
+ public createHash<E extends keyof IHashFormat>(data: string, hashFormat: E): IHashFormat[E] {
+ const hash = createHash('sha512').update(data).digest('hex');
+ if (hashFormat === 'number') {
+ const result = parseInt(hash, 16);
+ if (isNaN(result)) {
+ traceError(`Number hash for data '${data}' is NaN`);
+ }
+ return result as any;
+ }
+ return hash as any;
  }
 }

src/client/common/experiments.ts

@@ -148,7 +148,7 @@ export class ExperimentsManager implements IExperimentsManager {
  if (typeof (this.appEnvironment.machineId) !== 'string') {
  throw new Error('Machine ID should be a string');
  }
- const hash = this.crypto.createHash(`${this.appEnvironment.machineId}+${salt}`, 'hex', 'number');
+ const hash = this.crypto.createHash(`${this.appEnvironment.machineId}+${salt}`, 'number');
  return hash % 100 >= min && hash % 100 < max;
  }
 

src/client/common/types.ts

@@ -2,7 +2,6 @@
 // Licensed under the MIT License.
 'use strict';
 
-import { HexBase64Latin1Encoding } from 'crypto';
 import { Socket } from 'net';
 import { Request as RequestResult } from 'request';
 import { ConfigurationTarget, DiagnosticSeverity, Disposable, DocumentSymbolProvider, Event, Extension, ExtensionContext, OutputChannel, Uri, WorkspaceEdit } from 'vscode';
@@ -485,10 +484,9 @@ export interface ICryptoUtils {
  * Creates hash using the data and encoding specified
  * @returns hash as number, or string
  * @param data The string to hash
- * @param encoding Data encoding to use
  * @param hashFormat Return format of the hash, number or string
  */
- createHash<E extends keyof IHashFormat>(data: string, encoding: HexBase64Latin1Encoding, hashFormat: E): IHashFormat[E];
+ createHash<E extends keyof IHashFormat>(data: string, hashFormat: E): IHashFormat[E];
 }
 
 export const IAsyncDisposableRegistry = Symbol('IAsyncDisposableRegistry');

src/test/common/crypto.unit.test.ts

@@ -12,11 +12,37 @@ suite('Crypto Utils', async () => {
  crypto = new CryptoUtils();
  });
  test('If hashFormat equals `number`, method createHash() returns a number', async () => {
- const hash = crypto.createHash('blabla', 'hex', 'number');
+ const hash = crypto.createHash('blabla', 'number');
  assert.typeOf(hash, 'number', 'Type should be a number');
  });
  test('If hashFormat equals `string`, method createHash() returns a string', async () => {
- const hash = crypto.createHash('blabla', 'hex', 'string');
+ const hash = crypto.createHash('blabla', 'string');
  assert.typeOf(hash, 'string', 'Type should be a string');
  });
+ test('If hashFormat equals `number`, the hash should not be NaN', async () => {
+ let hash = crypto.createHash('test', 'number');
+ assert.isNotNaN(hash, 'Number hash should not be NaN');
+ hash = crypto.createHash('hash', 'number');
+ assert.isNotNaN(hash, 'Number hash should not be NaN');
+ hash = crypto.createHash('HASH1', 'number');
+ assert.isNotNaN(hash, 'Number hash should not be NaN');
+ });
+ test('If hashFormat equals `string`, the hash should not be undefined', async () => {
+ let hash = crypto.createHash('test', 'string');
+ assert.isDefined(hash, 'String hash should not be undefined');
+ hash = crypto.createHash('hash', 'string');
+ assert.isDefined(hash, 'String hash should not be undefined');
+ hash = crypto.createHash('HASH1', 'string');
+ assert.isDefined(hash, 'String hash should not be undefined');
+ });
+ test('If hashFormat equals `number`, hashes with different data should return different number hashes', async () => {
+ const hash1 = crypto.createHash('hash1', 'number');
+ const hash2 = crypto.createHash('hash2', 'number');
+ assert.notEqual(hash1, hash2, 'Hashes should be different numbers');
+ });
+ test('If hashFormat equals `string`, hashes with different data should return different string hashes', async () => {
+ const hash1 = crypto.createHash('hash1', 'string');
+ const hash2 = crypto.createHash('hash2', 'string');
+ assert.notEqual(hash1, hash2, 'Hashes should be different strings');
+ });
 });

src/test/common/experiments.unit.test.ts

@@ -599,10 +599,10 @@ suite('A/B experiments', () => {
  expect(() => expManager.isUserInRange(79, 94, 'salt')).to.throw();
  } else if (testParams.error) {
  const error = new Error('Kaboom');
- when(crypto.createHash(anything(), 'hex', 'number')).thenThrow(error);
+ when(crypto.createHash(anything(), 'number')).thenThrow(error);
  expect(() => expManager.isUserInRange(79, 94, 'salt')).to.throw(error);
  } else {
- when(crypto.createHash(anything(), 'hex', 'number')).thenReturn(testParams.hash);
+ when(crypto.createHash(anything(), 'number')).thenReturn(testParams.hash);
  expect(expManager.isUserInRange(79, 94, 'salt')).to.equal(testParams.expectedResult, 'Incorrectly identified');
  }
  });
@@ -636,7 +636,7 @@ suite('A/B experiments', () => {
  .returns(() => testParams.experimentStorageValue);
  when(appEnvironment.machineId).thenReturn('101');
  if (testParams.hash) {
- when(crypto.createHash(anything(), 'hex', 'number')).thenReturn(testParams.hash);
+ when(crypto.createHash(anything(), 'number')).thenReturn(testParams.hash);
  }
  expManager.populateUserExperiments();
  assert.deepEqual(expManager.userExperiments, testParams.expectedResult);