Re-factor code python execution framework #345

DonJayamanne · 2017-12-04T19:56:05Z

Ensure we have a generic way of executing python code, modules with support for env variables
Improvements to gulp to speed up compilation and improved dev experience
Added dependency injection
Modifications to unit tests framework (running pytests, nose, etc in extension) to make use of the new python execution engine along with DI
Code refactoring
- Move types into types.ts file
- Ensure all promises have exception handlers
Use latest version of typescript

Archive of 0.7.0

* 'master' of https://github.com/Microsoft/vscode-python: Fixes #56 list all environments (#219) Fixes #57 Disable activation on debugging (#220) Fixes #26 Do not run linters when linters are disabled (#222)

* upstream/master: Fix typo in README.md (#252) Disable linter without workspaces (#241)

* upstream/master: Document contribution to the code along with coding standards (#321)

* upstream/master: Add Simplified Chinese translation of commands (#240)

brettcannon · 2017-12-04T21:44:04Z

Is this all the work instead of just the tests?

DonJayamanne · 2017-12-04T22:04:29Z

Yes this is, but scooped just around the feature support of running pytest, nose and unit test in the extension,
Other PRs will follow with changes around formation, linting and intellisense and musc (sort imports, etc).

brettcannon · 2017-12-04T23:12:58Z

.vscode/tasks.json

@@ -47,6 +47,26 @@
                "isDefault": true
            }
        },
+        {
+            // Task that will replace 'Compile' and 'Hygiene', after having been battle-tested.


"after having been" -> "after being"

Sorry my linguistic curiosity, but could you please explain the difference to a non-native speaker? "after having been" sounded fine to me...

Yup, Bretts suggestion, makes sense to me.

brettcannon · 2017-12-04T23:28:04Z

src/client/common/types.ts

@@ -1,155 +1,18 @@
-/*---------------------------------------------------------------------------------------------


Why the removal of the copyright notice?

brettcannon · 2017-12-04T23:31:28Z

src/client/common/variables/types.ts

+
+/**
+ * An interface for a JavaScript object that
+ * acts a dictionary. The keys are strings.


"acts a" -> "acts as a"

brettcannon · 2017-12-04T23:32:05Z

src/client/common/variables/types.ts

+ * An interface for a JavaScript object that
+ * acts a dictionary. The keys are strings.
+ */
+export interface IStringDictionary<V> {


TS doesn't support this by default on the object type?

But that won't be strongly typed, this ensures properties are strings and values are of type V.

brettcannon · 2017-12-04T23:34:07Z

src/client/linters/pylama.ts

-
-                resolve(messages);
+        return this.run(pylamaPath, pylamaArgs.concat(['--format=parsable', document.uri.fsPath]), document, this.getWorkspaceRootPath(document), cancellation, REGEX).then(messages => {
+            // All messages in pylama are treated as warnings for now


Missing a period.

brettcannon · 2017-12-04T23:35:47Z

src/client/providers/lintProvider.ts

-                    }
-                    // set all diagnostics found in this pass, as this method always clears existing diagnostics.
-                    this.diagnosticCollection.set(document.uri, diagnostics);
+                        // Build the message and suffix the message with the name of the linter used


brettcannon · 2017-12-04T23:35:58Z

src/client/providers/lintProvider.ts

-                    this.diagnosticCollection.set(document.uri, diagnostics);
+                        // Build the message and suffix the message with the name of the linter used
+                        msgs.forEach(d => {
+                            // ignore magic commands from jupyter


Capitalize and period.

brettcannon · 2017-12-04T23:36:08Z

src/client/providers/lintProvider.ts

+                            diagnostics.push(createDiagnostics(d, document));
+                        });
+
+                        // Limit the number of messages to the max value


brettcannon · 2017-12-04T23:44:08Z

src/client/unittests/nosetest/services/parserService.ts

+                // We need to display the path relative to the current directory.
+                fileName = fileName.substring(rootDirectory.length + 1);
+                // we don't care about the compiled file.
+                if (path.extname(fileName) === '.pyc') {


Do we need to worry about .pyo files as well?

brettcannon · 2017-12-04T23:54:58Z

src/test/unittests/unittest.discovery.test.ts

+        procService.onExecObservable((file, args, options, callback) => {
+            if (args.length > 1 && args[0] === '-c' && args[1].includes('import unittest') && args[1].includes('loader = unittest.TestLoader()')) {
+                callback({
+                    // Ensure any spaces added during code formatting or the like are removed


DonJayamanne · 2017-12-05T17:28:59Z

@brettcannon fixed.

* upstream/master: Fix package.json (#347)

MikhailArkhipov · 2017-12-05T17:55:45Z

src/client/common/configSettings.ts

@@ -178,7 +179,7 @@ export class PythonSettings extends EventEmitter implements IPythonSettings {
    }
    // tslint:disable-next-line:function-name
    public static dispose() {
-        if (!IS_TEST_EXECUTION) {
+        if (!isTestExecution()) {


Alternative might be to define IPythonSettingsTest with additional test methods so regular code does not see test functions. Or derive a class like PythonSettingsTest with test methods and provide different factories to test and regular code.

Just a thought.

Totally agreed, unfortunately that's a bigger chunk of work.
The DI layer in this PR, and the existing IPythonSettings interface should help with this (its just a matter of refactoring stuff).
However I'd defer that to a later PR

* upstream/master: #34, #110 - suppress Intellisense in strings and comments (#339) Re-factor code python execution framework (#345)

octref and others added 30 commits November 3, 2017 13:11

Fix microsoft/vscode#37627 (#1368)

ecc1ca9

Version 0.7.0 of extension (#1381)

7c5778c

Archive of 0.7.0

Update README.md

9d1bf82

Update README.md

ffba179

sync fork with upstream

905c713

fix readme

acc2109

Merge branch 'master' of https://github.com/Microsoft/vscode-python

d470523

* 'master' of https://github.com/Microsoft/vscode-python: Fixes #56 list all environments (#219) Fixes #57 Disable activation on debugging (#220) Fixes #26 Do not run linters when linters are disabled (#222)

run without debugging

d32a546

send terminated event when process ends and fix linters

3f14d0a

gracefully handle program termination

f37803f

use vscode infrastructure to launch code in terminals

d69940e

merged upstream

d392e8b

merged

11c9514

ensure to check if launchArgs is not null

38befc0

ensure to check if launchArgs is not null

bea91fc

ensure to check if launchArgs is not null

0bb9054

fix code review comments

a105a20

fixed code review comments

8b2fd7c

Merge branch 'master' into RunWithoutDebug

5a7d21e

Merge branch 'master' into RunWithoutDebug

acd90c5

fixes #250, do not copy all env variables

09b3359

bug fix, launch using vscode and then fallback

9616eef

fix linter issue

f9fe2a8

platform process services

4963632

tests for decoder

edbe63e

tests for process service

47acd08

test to check source

284e221

tests for process utils

b27496b

Merge remote-tracking branch 'upstream/master'

92f775f

* upstream/master: Fix typo in README.md (#252) Disable linter without workspaces (#241)

fix imports

23fe3da

DonJayamanne added 15 commits December 1, 2017 20:36

removed unused method

9d6194b

speed up gulp

34158ba

removed unwanted setting

c823315

ensure chai 4.0.6 is used in tsd

e349e56

fixes

a1fea85

bitwise operations are allowed

cb0a354

Merge remote-tracking branch 'upstream/master'

f0f5c59

* upstream/master: Document contribution to the code along with coding standards (#321)

merged

c464a2a

enforce eol in vscode

011dca1

enforce new line

bfb8036

removed ts coding conventions

761f057

Merge remote-tracking branch 'upstream/master'

b2b9da9

* upstream/master: Add Simplified Chinese translation of commands (#240)

Use spaces instead of tabs

51b6c81

merged master

7fd36fc

added missing license headers and fix linter warnings

ada46ba

brettcannon suggested changes Dec 4, 2017

View reviewed changes

DonJayamanne added 2 commits December 5, 2017 09:23

fix code comments

c5e2d5d

fix code comments

b756ee8

DonJayamanne added 2 commits December 5, 2017 09:34

Merge remote-tracking branch 'upstream/master'

30a4091

* upstream/master: Fix package.json (#347)

merged

de14c78

MikhailArkhipov reviewed Dec 5, 2017

View reviewed changes

brettcannon approved these changes Dec 5, 2017

View reviewed changes

DonJayamanne merged commit 7efb558 into microsoft:master Dec 5, 2017

DonJayamanne mentioned this pull request Dec 9, 2017

Clean up tool execution in Unit Tests #350

Closed

DonJayamanne deleted the FixCodeExec branch December 12, 2017 21:21

DonJayamanne added a commit that referenced this pull request Dec 14, 2017

Merge remote-tracking branch 'upstream/master'

b16d2f9

* upstream/master: #34, #110 - suppress Intellisense in strings and comments (#339) Re-factor code python execution framework (#345)

lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-factor code python execution framework #345

Re-factor code python execution framework #345

DonJayamanne commented Dec 4, 2017

brettcannon commented Dec 4, 2017

DonJayamanne commented Dec 4, 2017

brettcannon Dec 4, 2017

balta2ar Dec 5, 2017

DonJayamanne Dec 5, 2017

brettcannon Dec 4, 2017

DonJayamanne Dec 5, 2017

DonJayamanne Dec 5, 2017

brettcannon Dec 4, 2017

brettcannon Dec 4, 2017

DonJayamanne Dec 5, 2017

brettcannon Dec 4, 2017

brettcannon Dec 4, 2017

brettcannon Dec 4, 2017

brettcannon Dec 4, 2017

brettcannon Dec 4, 2017

DonJayamanne Dec 5, 2017

brettcannon Dec 4, 2017

DonJayamanne commented Dec 5, 2017

MikhailArkhipov Dec 5, 2017

DonJayamanne Dec 5, 2017

		@@ -1,155 +1,18 @@
		/*---------------------------------------------------------------------------------------------

Re-factor code python execution framework #345

Re-factor code python execution framework #345

Conversation

DonJayamanne commented Dec 4, 2017

brettcannon commented Dec 4, 2017

DonJayamanne commented Dec 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DonJayamanne commented Dec 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment