Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add client tests #29

Merged
merged 11 commits into from
Oct 20, 2023
Merged

Add client tests #29

merged 11 commits into from
Oct 20, 2023

Conversation

deribaucourt
Copy link
Member

No description provided.

@deribaucourt deribaucourt changed the base branch from master to staging October 18, 2023 08:54
@deribaucourt deribaucourt force-pushed the add-client-tests branch 2 times, most recently from 41bc82a to 4657d51 Compare October 19, 2023 11:52
@deribaucourt deribaucourt marked this pull request as ready for review October 19, 2023 11:53
// under MIT License

const languages = {
createDiagnosticCollection: jest.fn(),
Copy link
Member

Choose a reason for hiding this comment

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

indentation is weird here

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed in another comment, I suggest keeping the original file and not changing it's linting rules.

Default: 0,
};

export = {
Copy link
Member

Choose a reason for hiding this comment

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

On my side typescript complains it is using the CommonJS syntax, while the EcmaScript syntaxt should be used (export default). Yet it won't compile if I make that change.

I am not sure where is exactly the problem, but I noticed .eslintrc.js is also using a commonjs export. I also noticed you put this file into "ignorePatterns". I am thinking this was an attempt to solve that problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

This file comes from another repo and works fine. As long as we don't change it, it makes no sense to me to test it's linting, which follows a different standard in another project. Also it makes it easy to update this file in the future when the vscode API changes/adds new features. I suggest to ignore linting this file and keep it as original. A comment at the top clearly indicates where it comes from.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. However I am surprised it works. Both syntaxes are not supposed to be interchangeable within a project. As a proof, this file does not compile if you use the ecmascript syntax. It makes me think the "root project" is not configured properly.

@deribaucourt deribaucourt force-pushed the add-client-tests branch 3 times, most recently from 10013d5 to fcbd549 Compare October 20, 2023 08:54
Using the Jest extension in VSCode, it is now possible to run, watch
and debug jest tests.
Add a dummy test to make sure the client test suite is working.
In order to make the class testable, we will need to mock vscode
before the global instance is created.
This is a mock of the VSCode API that is used by the Jest extension. It
is copied from the jest-community/vscode-jest project, which is licensed
under the MIT license.
The eslint plugin is also added to package.json. package-lock.json is
updated with npm 20.8.0.
The package*.json where updated with node 20.8.0.
Allows debugging the tests through the VSCode debugger. It was already
possible through the Jest extension.
@deribaucourt deribaucourt merged commit dd5cb91 into staging Oct 20, 2023
1 check passed
connection.onInitialize(async (params): Promise<InitializeResult> => {
const workspaceRoot = params.rootPath ?? ''
connection.onInitialize(async (params: InitializeParams): Promise<InitializeResult> => {
const workspaceRoot = params.workspaceFolders?.[0]?.uri ?? ''
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is possible that users have multiple folders opened in one workspace and all of them are bitbake projects.

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.

3 participants