Skip to content

Conversation

@bobwalker99
Copy link

Fixes #16368

Copy link
Member

@karthiknadig karthiknadig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Thanks for taking the time to update tests and linting.

@karthiknadig karthiknadig requested a review from kimadeline June 3, 2021 04:09
@bobwalker99
Copy link
Author

Hi I'm struggling to get the failing tests to fail locally and I can't understand how to re-run the failing PR/CI checks on github - is there any guidance you can point me at other than the 'contributing' wiki? Thanks, Bob

@karthiknadig
Copy link
Member

@bobwalker99 You many not have the permissions to run them. I will trigger a re-run.

@bobwalker99
Copy link
Author

Thanks @karthiknadig, much appreciated.

I think the failures highlighted by my change are partly due to this bit of code in the test setup:

out = out.replace(
/\/Users\/donjayamanne\/.vscode\/extensions\/pythonVSCode\/src\/test\/pythonFiles/g,
PYTHON_FILES_PATH,
);

The test is creating hybrid posix / windows paths which accidentally worked before in the test, but wouldn't happen in real life. I'm not sure whether the right thing to do is to try to just change the test, or try to handle it in the discovery parsing code. Both are potentially problematic.

It's also quite difficult to evaluate locally as I can't get the test grep to work to the point where I can just run this single test quickly and step though it, so I'm having to guess at it to a large extent. When I run the larger suite I get various other errors, including errors about missing nose and pytest, which are present in my activated virtualenv. I can't see where it decides which version of python to use for the tests?

All the best,
Bob

Copy link

@paulacamargo25 paulacamargo25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice Job :)

@karthiknadig
Copy link
Member

@bobwalker99 You probably have to set CI_PYTHON_PATH=<python path> for these tests to work correctly.

This configuration below worked for me to narrow down to the nose test suite:

{
            "name": "Tests (Single Workspace, VS Code, *.test.ts)",
            "type": "extensionHost",
            "request": "launch",
            "runtimeExecutable": "${execPath}",
            "args": [
                "${workspaceFolder}/src/test",
                "--disable-extensions",
                "--extensionDevelopmentPath=${workspaceFolder}",
                "--extensionTestsPath=${workspaceFolder}/out/test"
            ],
            "env": {
                "VSC_PYTHON_CI_TEST_GREP": "Unit Tests - nose"
            },
            "stopOnEntry": false,
            "sourceMaps": true,
            "outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"],
            "preLaunchTask": "Compile",
            "skipFiles": ["<node_internals>/**"]
        },

@karthiknadig
Copy link
Member

Can you rebase with the latest main? I can look into the tests and see what needs to be changed.

@bobwalker99
Copy link
Author

Can you rebase with the latest main? I can look into the tests and see what needs to be changed.

Done, thanks @karthiknadig

Also match on unmodified input to forgive hybrid separators scenario in Windows tests (thanks @karthiknadig)

Co-authored-by: Karthik Nadig <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nose and Unittest tests display full path on every node in test explorer

4 participants