-
Notifications
You must be signed in to change notification settings - Fork 25
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
test: Improve test coverage of Language Server component #61
Conversation
} | ||
return result; | ||
std::lock_guard<std::mutex> lock(q_mtx_); | ||
return !requests_.empty(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note, I did exactly this in the past and for some reason it was breaking the server... As long as it works now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any idea under what conditions it used to break? Because I am pretty sure these two implementations are equivalent.
CMakeSettings.json
Outdated
"name": "WSL-Clang-Release-ASAN", | ||
"generator": "Ninja", | ||
"configurationType": "Release", | ||
"buildRoot": "${projectDir}\\out\\build\\${name}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we unify build & install root dir for x64/86 and WSL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where should be that unified location?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the one for the x64/86 was OK IMO.
result = !requests_.empty(); | ||
} | ||
return result; | ||
std::lock_guard<std::mutex> lock(q_mtx_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think that this lock is redundant. the whole method seems suspicious. ideally, you want to put lock in some if statement rather then in reading bool
{
std::lock_guard<std::mutex> lock(q_mtx_);
if (is_running())
...
}
@@ -64,12 +70,8 @@ void request_manager::end_worker() | |||
|
|||
bool hlasm_plugin::language_server::request_manager::is_running() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool hlasm_plugin::language_server::request_manager::is_running() | |
bool request_manager::is_running() |
Kudos, SonarCloud Quality Gate passed! 0 Bugs |
🎉 This PR is included in version 0.12.0-beta.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
🎉 This PR is included in version 0.12.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
…4z#61) * refactoring & dispatcher tests * dispatcher test * add feature launch test (so far launch, stackTrace and setBreakpoints tested) * test: finished launch feature tests * refactor: refactor register methods so virtual method is not called in constructors * refactor: fix sonarcloud code smells * refactor: fix code smells * test: add dap server test * test: improve dispatcher and lsp_erver tests * test: add malformed message test for dap_server * refactor: fix exception thowing * test: variable initialization * test: fix never ending test * avoid waiting infinitely when there are no requests * test: add finish requests test * test: fix finish requests test * fix possible race condition * remove unnecessary lock * unify visual studio build folders * code smell fixes
No description provided.