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

Fail hard when C# and VB.NET plugins are missing on the server #1787

Merged

Conversation

cristian-ambrosini-sonarsource
Copy link
Contributor

Fixes #1475

// Arrange
var localCacheDir = TestUtils.CreateTestSpecificFolderWithSubPaths(TestContext);
var logger = new TestLogger();

var request1 = new Plugin("no.matching.resource.plugin", "2.0", "non.existent.resource.zip");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed: no matching plugin is causing hard failure (already tested in EmbeddedInstall_MissingResource_ThrowFileNotFoundException).

@@ -95,6 +80,7 @@ public void EmbeddedInstall_SinglePlugin_SingleResource_Succeeds()
var actualFiles = testSubject.InstallAssemblies(new Plugin[] { requestedPlugin });

// Assert
logger.AssertInfoLogged("Processing plugin: plugin1 version 1.0");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Assertion moved from InstallAssemblies_ValidPlugins_PluginsVersionLogInfo.
The previous test was not feasible anymore due to the introduction of the new exception being thrown when plugins are missing.

Copy link
Contributor

@antonioaversa antonioaversa left a comment

Choose a reason for hiding this comment

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

LGTM

There is a Code Smell failing the Quality Gate, that needs attention.

{
const string missingPluginKey = "missingPlugin";
const string missingPluginVersion = "1.0";
const string missingPluginResource = "resource.txt";
Copy link
Contributor

Choose a reason for hiding this comment

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

Personal opinion: I liked could.be.anything. It conveys the information that the name of the plugin resource is not relevant for the test... and could be anything!

Copy link

sonarcloud bot commented Nov 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@antonioaversa
Copy link
Contributor

antonioaversa commented Nov 23, 2023

LGTM

There is a Code Smell failing the Quality Gate, that needs attention.

As discussed offline, the Code Smell mentioned above (new link here) has been solved as "Won't fix" with the following reasoning:

The EmbeddedAnalyzerInstaller uses a downloader underneath. The downloader has an async interface, that doesn't make sense to change.
At the same time EmbeddedAnalyzerInstaller, or its caller needs to run synchronously.
Therefore, the async chain needs to be broken, and this method seems one of the most appropriate locations to do so.

@cristian-ambrosini-sonarsource CI can be re-run.

@cristian-ambrosini-sonarsource cristian-ambrosini-sonarsource merged commit 305266f into master Nov 23, 2023
21 checks passed
@cristian-ambrosini-sonarsource cristian-ambrosini-sonarsource deleted the cristian/fail-hard-missing-plugin branch November 23, 2023 11:29
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.

Fail hard when C# and VB.NET plugins are missing on the server
3 participants