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.

Sign up for GitHub

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

PackageManagementFormat only updates settings when it has a value #6016

Merged
merged 2 commits into from
Sep 10, 2024
Merged

PackageManagementFormat only updates settings when it has a value #6016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
20b3aa0
PackageManagementFormat only updates settings when it has a value
donnie-msft Sep 6, 2024
11cd3f5
Add tests
donnie-msft Sep 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/NuGet.Clients/NuGet.VisualStudio.Common/PackageManagementFormat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class PackageManagementFormat
// keep track of current value for selected package format
private int _selectedPackageFormat = -1;

// keep track of current value for shwo dialog checkbox
// keep track of current value for show dialog checkbox
private bool? _showDialogValue;

public PackageManagementFormat(Configuration.ISettings settings)
Expand Down Expand Up @@ -102,10 +102,17 @@ public int SelectedPackageManagementFormat

public void ApplyChanges()
{
_settings.AddOrUpdate(ConfigurationConstants.PackageManagementSection,
new AddItem(ConfigurationConstants.DefaultPackageManagementFormatKey, _selectedPackageFormat.ToString(CultureInfo.InvariantCulture)));
_settings.AddOrUpdate(ConfigurationConstants.PackageManagementSection,
new AddItem(ConfigurationConstants.DoNotShowPackageManagementSelectionKey, _showDialogValue.Value.ToString(CultureInfo.InvariantCulture)));
if (_selectedPackageFormat != -1)
Copy link
Contributor

Choose a reason for hiding this comment

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

To guard against any future code changes that could use another sentinel value (like -2, etc), you could make this:

Suggested change
if (_selectedPackageFormat != -1)
if (_selectedPackageFormat > -1)

{
_settings.AddOrUpdate(ConfigurationConstants.PackageManagementSection,
new AddItem(ConfigurationConstants.DefaultPackageManagementFormatKey, _selectedPackageFormat.ToString(CultureInfo.InvariantCulture)));
}

if (_showDialogValue.HasValue)
{
_settings.AddOrUpdate(ConfigurationConstants.PackageManagementSection,
new AddItem(ConfigurationConstants.DoNotShowPackageManagementSelectionKey, _showDialogValue.Value.ToString(CultureInfo.InvariantCulture)));
}
_settings.SaveToDisk();
}

Expand Down
78 changes: 78 additions & 0 deletions test/NuGet.Clients.Tests/NuGet.VisualStudio.Common.Test/PackageManagementFormatTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Globalization;
using Moq;
using NuGet.Configuration;
using Xunit;

namespace NuGet.VisualStudio.Common.Test
{
public class PackageManagementFormatTests
{
private Mock<ISettings> _settings;
private PackageManagementFormat _packageManagementFormat;

public PackageManagementFormatTests()
{
_settings = new Mock<ISettings>();
_packageManagementFormat = new PackageManagementFormat(_settings.Object);
}

[Fact]
public void ApplyChanges_WhenPropertiesNotChanged_DoesNotThrowOrUpdateSettings()
{
// Act
_packageManagementFormat.ApplyChanges();

// Assert
// Ensure no settings change calls are made.
_settings.Verify(settings => settings.AddOrUpdate(It.IsAny<string>(), It.IsAny<SettingItem>()), Times.Never);

// Calling to save settings should always happen, regardless of values.
_settings.Verify(settings => settings.SaveToDisk(), Times.Once());
}

[Fact]
public void ApplyChanges_WhenEnabledIsSet_SavesAndDoesNotThrow()
{
// Arrange
_packageManagementFormat.Enabled = false;
string expectedBooleanAsString = false.ToString(CultureInfo.InvariantCulture);

// Act
_packageManagementFormat.ApplyChanges();

// Assert
_settings.Verify(settings => settings.AddOrUpdate(ConfigurationConstants.PackageManagementSection,
It.Is<AddItem>(addItem => addItem.Key == ConfigurationConstants.DoNotShowPackageManagementSelectionKey && addItem.Value == expectedBooleanAsString)),
Times.Once);

// Calling to save settings should always happen, regardless of values.
_settings.Verify(settings => settings.SaveToDisk(), Times.Once());

_settings.VerifyNoOtherCalls();
}

[Fact]
public void ApplyChanges_WhenSelectedPackageManagementFormatIsSet_SavesAndDoesNotThrow()
{
// Arrange
_packageManagementFormat.SelectedPackageManagementFormat = 0;
string expectedIntegerAsString = 0.ToString(CultureInfo.InvariantCulture);

// Act
_packageManagementFormat.ApplyChanges();

// Assert
_settings.Verify(settings => settings.AddOrUpdate(ConfigurationConstants.PackageManagementSection,
It.Is<AddItem>(addItem => addItem.Key == ConfigurationConstants.DefaultPackageManagementFormatKey && addItem.Value == expectedIntegerAsString)),
Times.Once);

// Calling to save settings should always happen, regardless of values.
_settings.Verify(settings => settings.SaveToDisk(), Times.Once());

_settings.VerifyNoOtherCalls();
}
}
}