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 unit tests for Java, split test cases per language #42

Conversation

ResistantBear
Copy link
Collaborator

@ResistantBear ResistantBear commented Jan 23, 2024

Added unit tests for basic file metrics for java source code. Also introduces a new function to reduce code duplication inside the test case. Splits the test cases of GenericParser.test.ts into multiple test files, to have one test file for each programming language.

MCC is not yet tested due to the unclear definition of that metric (see #41).

Main author @mylinhdao

Closes #40

@ResistantBear ResistantBear linked an issue Jan 23, 2024 that may be closed by this pull request
System.out.println("Iteration " + (counter + 1));
counter++;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • do while,
  • for(var item : items),
  • what about Iterable.foreach(() => {}). How is this case handled e.g. in Sonar?

Copy link
Collaborator Author

@ResistantBear ResistantBear Jan 29, 2024

Choose a reason for hiding this comment

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

According to https://docs.sonarsource.com/sonarqube/latest/user-guide/metric-definitions/, SonarQube does increase the complexity by one for "if, for, while, case, &&, ||, ?, ->". That means that in your example, it would actually increase the complexity by one, but not because of the "foreach"-call itself, but because of the lambda function with the arrow operator. Calling foreach with some non-lambda function should not do anything accordingly.

I guess that whole discussion should be part of #41, however

Copy link
Collaborator

Choose a reason for hiding this comment

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

This question is not blocking the extension of more cases for control structures in this code file.

test/parser/TestHelper.ts Outdated Show resolved Hide resolved
test/parser/TestHelper.ts Outdated Show resolved Hide resolved
@ce-bo
Copy link
Collaborator

ce-bo commented Jan 25, 2024

PTAL, in general LGTM.
Please create issue or link existing one for adding mcc tests as soon as we agree upon a metric definition.

@ResistantBear ResistantBear changed the title Add unit tests for Java Add unit tests for Java, Split test cases per language Jan 25, 2024
@ResistantBear ResistantBear changed the title Add unit tests for Java, Split test cases per language Add unit tests for Java, split test cases per language Jan 25, 2024
@ResistantBear ResistantBear marked this pull request as ready for review January 26, 2024 12:59
@ce-bo
Copy link
Collaborator

ce-bo commented Jan 30, 2024

I guess this is still in progress, so I moved it into the right state :-)

@ResistantBear ResistantBear force-pushed the 8-stabilize-supported-programming-languages-by-adding-automated-tests branch from 6f33593 to 10363b9 Compare January 30, 2024 15:50
@ResistantBear
Copy link
Collaborator Author

We have encountered an issue where some tests "just fail" because of tree.rootNode being undefined when running all tests (both test.ts files and compiled .test.js files) e.g. via "npm test". This seems to be triggered by the higher number of test files or test cases in this PR, but also happened seldomly before.

As it turns out, this is probably caused by a race condition in the tree-sitter library.
See tree-sitter/node-tree-sitter#181
There is a potential fix for this linked, but it never got merged:
tree-sitter/node-tree-sitter#75

We probably cannot do anything here to handle this issue on our side...

@ResistantBear ResistantBear force-pushed the 8-stabilize-supported-programming-languages-by-adding-automated-tests branch from dede3c2 to 006fb96 Compare January 31, 2024 12:55
@ResistantBear ResistantBear force-pushed the 8-stabilize-supported-programming-languages-by-adding-automated-tests branch from 67ba45b to 36dacf3 Compare February 1, 2024 11:28
@ResistantBear
Copy link
Collaborator Author

We have encountered an issue where some tests "just fail" because of tree.rootNode being undefined when running all tests (both test.ts files and compiled .test.js files) e.g. via "npm test". This seems to be triggered by the higher number of test files or test cases in this PR, but also happened seldomly before.

As it turns out, this is probably caused by a race condition in the tree-sitter library. See tree-sitter/node-tree-sitter#181 There is a potential fix for this linked, but it never got merged: tree-sitter/node-tree-sitter#75

We probably cannot do anything here to handle this issue on our side...

It turned out that this issue is triggered especially if the number of test files is higher than the number of worker processes used by jest.
As a (temporal) mitigation for this issue, I reduced the number of test files by excluding the compiled .js files in the /dist/test folder from running with jest, as well as setting --maxWorkers to 24.

@ResistantBear ResistantBear force-pushed the 8-stabilize-supported-programming-languages-by-adding-automated-tests branch from 151bedd to 4316cef Compare February 2, 2024 09:58
Copy link
Collaborator Author

@ResistantBear ResistantBear left a comment

Choose a reason for hiding this comment

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

Splitted out the issue that default statements in switch-case-blocks are counted for the complexity metric into #64
Splitted out issue caused by the race condition inside the tree-sitter library into #65

All other remarks are resolved.

Copy link
Collaborator

@mylinhdao mylinhdao left a comment

Choose a reason for hiding this comment

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

All remarks are solved.

@mylinhdao mylinhdao merged commit 4d6dad7 into main Feb 2, 2024
@mylinhdao mylinhdao deleted the 8-stabilize-supported-programming-languages-by-adding-automated-tests branch February 2, 2024 10:08
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.

Add tests for file metrics for Java
4 participants