Revert discovery of pytest by scraping the output (#5517)

* Revert
* Fix paths
* Update change log
* Fixes

Author: DonJayamanne

CHANGELOG.md

@@ -1,14 +1,21 @@
 # Changelog
 
-## 2019.4.1 (24 April 2019)
+## 2019.4.2 (29 April 2019)
+
+### Fixes
+
+1. Revert discovery of `pytest` tests to use the approach of scraping the output.
+   ([#5458](https://github.com/Microsoft/vscode-python/issues/5458))
+
+## 2019.4.11987 (24 April 2019)
 
 ### Fixes
 
 1. Remove trailing commas in JSON files.
    (thanks [Romain](https://github.com/quarthex))
    ([#5437](https://github.com/Microsoft/vscode-python/issues/5437))
 
-## 2019.4.0 (23 April 2019)
+## 2019.4.11881 (23 April 2019)
 
 ### Enhancements
 

src/client/telemetry/constants.ts

@@ -43,7 +43,6 @@ export enum EventName {
     UNITTEST_DISABLE = 'UNITTEST.DISABLE',
     UNITTEST_RUN = 'UNITTEST.RUN',
     UNITTEST_DISCOVER = 'UNITTEST.DISCOVER',
-    UNITTEST_DISCOVER_WITH_PYCODE = 'UNITTEST.DISCOVER.WITH.PYTHONCODE',
     UNITTEST_CONFIGURE = 'UNITTEST.CONFIGURE',
     UNITTEST_CONFIGURING = 'UNITTEST.CONFIGURING',
     UNITTEST_VIEW_OUTPUT = 'UNITTEST.VIEW_OUTPUT',

src/client/telemetry/index.ts

@@ -312,7 +312,6 @@ export interface IEventNamePropertyMapping {
     [EventName.UNITTEST_CONFIGURING]: TestConfiguringTelemetry;
     [EventName.TERMINAL_CREATE]: TerminalTelemetry;
     [EventName.UNITTEST_DISCOVER]: TestDiscoverytTelemetry;
-    [EventName.UNITTEST_DISCOVER_WITH_PYCODE]: never | undefined;
     [EventName.UNITTEST_RUN]: TestRunTelemetry;
     [EventName.UNITTEST_STOP]: never | undefined;
     [EventName.UNITTEST_DISABLE]: never | undefined;

src/client/testing/common/services/discovery.ts

@@ -360,6 +360,31 @@ function getParentTestFolder(tests: Tests, item: TestFolder | TestFile): TestFol
     return getParentTestFolderForFile(tests, item as TestFile);
 }
 
+/**
+ * Returns the parent test folder give a given test file.
+ *
+ * @param {Tests} tests
+ * @param {TestFile} file
+ * @returns {(TestFolder | undefined)}
+ */
+function getParentTestFolderForFile(tests: Tests, file: TestFile): TestFolder | undefined {
+    return tests.testFolders.find(folder => folder.testFiles.some(item => item === file));
+}
+
+/**
+ * Returns the parent test folder for a given test folder.
+ *
+ * @param {Tests} tests
+ * @param {TestFolder} folder
+ * @returns {(TestFolder | undefined)}
+ */
+function getParentTestFolderForFolder(tests: Tests, folder: TestFolder): TestFolder | undefined {
+    if (tests.rootTestFolders.indexOf(folder) >= 0) {
+        return;
+    }
+    return tests.testFolders.find(item => item.folders.some(child => child === folder));
+}
+
 /**
  * Gets the parent test file for a test item.
  *
@@ -395,31 +420,6 @@ export function getParentSuite(tests: Tests, suite: TestSuite | TestFunction): T
     return;
 }
 
-/**
- * Returns the parent test folder give a given test file.
- *
- * @param {Tests} tests
- * @param {TestFile} file
- * @returns {(TestFolder | undefined)}
- */
-function getParentTestFolderForFile(tests: Tests, file: TestFile): TestFolder | undefined {
-    return tests.testFolders.find(folder => folder.testFiles.some(item => item === file));
-}
-
-/**
- * Returns the parent test folder for a given test folder.
- *
- * @param {Tests} tests
- * @param {TestFolder} folder
- * @returns {(TestFolder | undefined)}
- */
-function getParentTestFolderForFolder(tests: Tests, folder: TestFolder): TestFolder | undefined {
-    if (tests.rootTestFolders.indexOf(folder) >= 0) {
-        return;
-    }
-    return tests.testFolders.find(item => item.folders.some(child => child === folder));
-}
-
 /**
  * Given a test function will return the corresponding flattened test function.
  *

src/client/testing/common/testUtils.ts

@@ -1,20 +1,23 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-import { inject, injectable } from 'inversify';
+import { inject, injectable, named } from 'inversify';
 import { CancellationTokenSource } from 'vscode';
 import { IServiceContainer } from '../../../ioc/types';
 import { PYTEST_PROVIDER } from '../../common/constants';
-import { ITestDiscoveryService, ITestsHelper, TestDiscoveryOptions, Tests } from '../../common/types';
+import { ITestDiscoveryService, ITestRunner, ITestsHelper, ITestsParser, Options, TestDiscoveryOptions, Tests } from '../../common/types';
 import { IArgumentsService, TestFilter } from '../../types';
 
 @injectable()
 export class TestDiscoveryService implements ITestDiscoveryService {
     private argsService: IArgumentsService;
     private helper: ITestsHelper;
-    constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) {
+    private runner: ITestRunner;
+    constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer,
+        @inject(ITestsParser) @named(PYTEST_PROVIDER) private testParser: ITestsParser) {
         this.argsService = this.serviceContainer.get<IArgumentsService>(IArgumentsService, PYTEST_PROVIDER);
         this.helper = this.serviceContainer.get<ITestsHelper>(ITestsHelper);
+        this.runner = this.serviceContainer.get<ITestRunner>(ITestRunner);
     }
     public async discoverTests(options: TestDiscoveryOptions): Promise<Tests> {
         const args = this.buildTestCollectionArgs(options);
@@ -39,7 +42,7 @@ export class TestDiscoveryService implements ITestDiscoveryService {
 
         return this.helper.mergeTests(results);
     }
-    protected buildTestCollectionArgs(options: TestDiscoveryOptions) {
+    private buildTestCollectionArgs(options: TestDiscoveryOptions) {
         // Remove unwnted arguments (which happen to be test directories & test specific args).
         const args = this.argsService.filterArguments(options.args, TestFilter.discovery);
         if (options.ignoreCache && args.indexOf('--cache-clear') === -1) {
@@ -48,19 +51,24 @@ export class TestDiscoveryService implements ITestDiscoveryService {
         if (args.indexOf('-s') === -1) {
             args.splice(0, 0, '-s');
         }
+        args.splice(0, 0, '--collect-only');
         return args;
     }
-    protected async discoverTestsInTestDirectory(options: TestDiscoveryOptions): Promise<Tests> {
+    private async discoverTestsInTestDirectory(options: TestDiscoveryOptions): Promise<Tests> {
         const token = options.token ? options.token : new CancellationTokenSource().token;
-        const discoveryOptions = { ...options };
-        discoveryOptions.args = ['discover', 'pytest', '--', ...options.args];
-        discoveryOptions.token = token;
+        const runOptions: Options = {
+            args: options.args,
+            cwd: options.cwd,
+            workspaceFolder: options.workspaceFolder,
+            token,
+            outChannel: options.outChannel
+        };
 
-        const discoveryService = this.serviceContainer.get<ITestDiscoveryService>(ITestDiscoveryService, 'common');
-        if (discoveryOptions.token && discoveryOptions.token.isCancellationRequested) {
+        const data = await this.runner.run(PYTEST_PROVIDER, runOptions);
+        if (options.token && options.token.isCancellationRequested) {
             return Promise.reject<Tests>('cancelled');
         }
 
-        return discoveryService.discoverTests(discoveryOptions);
+        return this.testParser.parse(data, options);
     }
 }