Skip to content

Conversation

@anthonykim1
Copy link

@anthonykim1 anthonykim1 commented Sep 21, 2024

Adding test for: #24111

@anthonykim1 anthonykim1 added debt Covers everything internal: CI, testing, refactoring of the codebase, etc. area-terminal labels Sep 21, 2024
@anthonykim1 anthonykim1 changed the title Start creating test for PYTHONSTARTUP setting Test for PYTHONSTARTUP setting Sep 21, 2024
@anthonykim1
Copy link
Author

anthonykim1 commented Sep 21, 2024

Need to resolve: Keep running into context.environmentVariableCollection.replace is not a function error:

Tried to follow logic in

collection = mock<EnvironmentVariableCollection>();
but still same error.

It seems that problem is I cannot access environmentVariableCollection from IExtensionContext ONLY when I'm in the test file.

@anthonykim1 anthonykim1 changed the title Test for PYTHONSTARTUP setting Tests for PYTHONSTARTUP setting Sep 21, 2024
@anthonykim1
Copy link
Author

anthonykim1 commented Sep 21, 2024

Note: All Three approach of trying to setup/mock EnvironmentCollection.replace is giving "Not a function error"

            globalEnvironmentVariableCollection
            .setup((c) => c.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
            .returns(() => Promise.resolve());

            environmentVariableCollection
            .setup((c) => c.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
            .returns(() => Promise.resolve());


           context
            .setup((c) =>
                c.environmentVariableCollection.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()),
            )
            .returns(() => Promise.resolve());

Edit:

   context
            .setup((c) =>
                c.environmentVariableCollection.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()),
            )
            .returns(() => Promise.resolve());

is a wrong way to set up when using typemoq. It may be right for ts-mockito but not for typemoq.

@anthonykim1 anthonykim1 marked this pull request as ready for review September 21, 2024 06:40
@vs-code-engineering vs-code-engineering bot added this to the September 2024 milestone Sep 21, 2024
import { VariablesProvider } from '../../client/repl/variables/variablesProvider';
import { VariableRequester } from '../../client/repl/variables/variableRequester';

suite.only('ReplVariablesProvider', () => {
Copy link
Member

Choose a reason for hiding this comment

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

We have a hygiene rule in core to prevent this slipping in

Copy link
Author

Choose a reason for hiding this comment

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

This is a good idea and I've made item on this: #24161
I see "local/code-no-test-only": "error", from vscode. Not sure if that local/code is something vscode repository specific, but I think something like:

{
  "plugins": ["mocha"],
  "rules": {
    "mocha/no-exclusive-tests": "error"
  }
}

works too

@anthonykim1 anthonykim1 merged commit 06a976f into microsoft:main Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-terminal debt Covers everything internal: CI, testing, refactoring of the codebase, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants