Skip to content

Commit

Permalink
Address cr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
bhsubra committed Sep 17, 2021
1 parent f81134b commit 699af2a
Show file tree
Hide file tree
Showing 14 changed files with 54 additions and 74 deletions.
46 changes: 14 additions & 32 deletions src/Bicep.Cli.IntegrationTests/BuildCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ public class BuildCommandTests : TestBase
[NotNull]
public TestContext? TestContext { get; set; }

private string CurrentDirectory = Directory.GetCurrentDirectory();

[TestMethod]
public async Task Build_ZeroFiles_ShouldFail_WithExpectedErrorMessage()
{
Expand Down Expand Up @@ -290,29 +288,25 @@ public async Task Build_LockedOutputFile_ShouldProduceExpectedError()
}

[TestMethod]
// ConfigHelper looks for bicepconfig.json in CurrentDirectory. We'll set the CurrentDirectory
// in this test. To avoid conflicts, we'll disable parallelation
[DoNotParallelize]
public async Task Build_WithEmptyBicepConfig_ShouldProduceExpectedError()
public async Task Build_WithEmptyBicepConfig_ShouldProduceOutputFile()
{
string testOutputPath = Path.Combine(TestContext.ResultsDirectory, Guid.NewGuid().ToString());
var inputFile = FileHelper.SaveResultFile(this.TestContext, "main.bicep", DataSets.Empty.Bicep, testOutputPath);
FileHelper.SaveResultFile(this.TestContext, "bicepconfig.json", string.Empty, testOutputPath);

Directory.SetCurrentDirectory(testOutputPath);

var (output, error, result) = await Bicep("build", inputFile);

result.Should().Be(1);
result.Should().Be(0);
output.Should().BeEmpty();
error.Should().Contain("main.bicep(1,1) : Error : Could not load configuration file. The input does not contain any JSON tokens. Expected the input to start with a valid JSON token, when isFinalBlock is true. LineNumber: 0 | BytePositionInLine: 0.");
error.Should().BeEmpty();

var expectedOutputFile = Path.Combine(testOutputPath, "main.json");

File.Exists(expectedOutputFile).Should().BeTrue();
}

[TestMethod]
// ConfigHelper looks for bicepconfig.json in CurrentDirectory. We'll set the CurrentDirectory
// in this test. To avoid conflicts, we'll disable parallelation
[DoNotParallelize]
public async Task Build_WithInvalidBicepConfig_ShouldProduceExpectedError()
public async Task Build_WithInvalidBicepConfig_ShouldProduceOutputFile()
{
string testOutputPath = Path.Combine(TestContext.ResultsDirectory, Guid.NewGuid().ToString());
var inputFile = FileHelper.SaveResultFile(this.TestContext, "main.bicep", DataSets.Empty.Bicep, testOutputPath);
Expand All @@ -326,19 +320,18 @@ public async Task Build_WithInvalidBicepConfig_ShouldProduceExpectedError()
""level"": ""info""
", testOutputPath);

Directory.SetCurrentDirectory(testOutputPath);

var (output, error, result) = await Bicep("build", inputFile);

result.Should().Be(1);
result.Should().Be(0);
output.Should().BeEmpty();
error.Should().Contain("main.bicep(1,1) : Error : Could not load configuration file. Expected depth to be zero at the end of the JSON payload. There is an open JSON object or array that should be closed. LineNumber: 8 | BytePositionInLine: 0.");
error.Should().BeEmpty();

var expectedOutputFile = Path.Combine(testOutputPath, "main.json");

File.Exists(expectedOutputFile).Should().BeTrue();
}

[TestMethod]
// ConfigHelper looks for bicepconfig.json in CurrentDirectory. We'll set the CurrentDirectory
// in this test. To avoid conflicts, we'll disable parallelation
[DoNotParallelize]
public async Task Build_WithValidBicepConfig_ShouldProduceExpectedError()
{
string testOutputPath = Path.Combine(TestContext.ResultsDirectory, Guid.NewGuid().ToString());
Expand All @@ -357,8 +350,6 @@ public async Task Build_WithValidBicepConfig_ShouldProduceExpectedError()
}
}", testOutputPath);

Directory.SetCurrentDirectory(testOutputPath);

var expectedOutputFile = Path.Combine(testOutputPath, "main.json");

File.Exists(expectedOutputFile).Should().BeFalse();
Expand All @@ -385,15 +376,6 @@ private static IEnumerable<object[]> GetValidDataSetsWithExternalModules() => Da
.AllDataSets
.Where(ds => ds.IsValid && ds.HasExternalModules)
.ToDynamicTestData();

[TestCleanup]
public void Cleanup()
{
if (!string.Equals(Directory.GetCurrentDirectory(), CurrentDirectory, StringComparison.OrdinalIgnoreCase))
{
Directory.SetCurrentDirectory(CurrentDirectory);
}
}
}
}

47 changes: 29 additions & 18 deletions src/Bicep.Cli/Services/CompilationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@
// Licensed under the MIT License.

using Bicep.Cli.Logging;
using Bicep.Core.Diagnostics;
using Bicep.Core.Configuration;
using Bicep.Core.Extensions;
using Bicep.Core.FileSystem;
using Bicep.Core.Parsing;
using Bicep.Core.Registry;
using Bicep.Core.Semantics;
using Bicep.Core.TypeSystem.Az;
using Bicep.Core.Workspaces;
using Bicep.Decompiler;
using System;
using System.Collections.Immutable;
using System.IO;
using System.Threading.Tasks;

namespace Bicep.Cli.Services
Expand Down Expand Up @@ -63,27 +64,37 @@ public async Task RestoreAsync(string inputPath)
}
}

try
{
var compilation = new Compilation(this.invocationContext.ResourceTypeProvider, sourceFileGrouping);
ConfigHelper configHelper = GetConfigHelper(inputUri);
var compilation = new Compilation(this.invocationContext.ResourceTypeProvider, sourceFileGrouping, configHelper);

LogDiagnostics(compilation);
LogDiagnostics(compilation);

return compilation;
}
catch (Exception exception)
{
var diagnostic = new Diagnostic(
new TextSpan(0, 0),
DiagnosticLevel.Error,
string.Empty,
exception.Message,
inputUri);
return compilation;
}

diagnosticLogger.LogDiagnostic(inputUri, diagnostic, sourceFileGrouping.EntryPoint.LineStarts);
private ConfigHelper GetConfigHelper(Uri uri)
{
ConfigHelper configHelper;

return null;
try
{
configHelper = new ConfigHelper(Path.GetDirectoryName(uri.LocalPath), fileResolver);
}
catch (Exception ex)
{
var invocationContext = new InvocationContext(
AzResourceTypeProvider.CreateWithAzTypes(),
Console.Out,
Console.Error,
ThisAssembly.AssemblyFileVersion,
features: null,
clientFactory: null);
invocationContext.OutputWriter.WriteLine(ex.Message);

configHelper = new ConfigHelper(null, fileResolver, useDefaultConfig: true).GetDisabledLinterConfig();
}

return configHelper;
}

public async Task<(Uri, ImmutableDictionary<Uri, string>)> DecompileAsync(string inputPath, string outputPath)
Expand Down
2 changes: 0 additions & 2 deletions src/Bicep.Core.IntegrationTests/RegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Bicep.Core.Configuration;
using Bicep.Core.Diagnostics;
using Bicep.Core.Features;
using Bicep.Core.FileSystem;
Expand All @@ -15,7 +14,6 @@
using Bicep.Core.TypeSystem.Az;
using Bicep.Core.UnitTests;
using Bicep.Core.UnitTests.Assertions;
using Bicep.Core.UnitTests.Configuration;
using Bicep.Core.UnitTests.Utils;
using Bicep.Core.Workspaces;
using FluentAssertions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using Bicep.Core.Semantics;
using Bicep.Core.UnitTests;
using Bicep.Core.UnitTests.Assertions;
using Bicep.Core.UnitTests.Configuration;
using Bicep.Core.UnitTests.Utils;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using Bicep.Core.TypeSystem;
using Bicep.Core.UnitTests;
using Bicep.Core.UnitTests.Assertions;
using Bicep.Core.UnitTests.Configuration;
using Bicep.Core.UnitTests.Utils;
using FluentAssertions;
using Microsoft.VisualStudio.TestTools.UnitTesting;
Expand Down
1 change: 0 additions & 1 deletion src/Bicep.Core.Samples/ExamplesTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Bicep.Core.Configuration;
using Bicep.Core.UnitTests.Configuration;
using Bicep.Core.Registry;

namespace Bicep.Core.Samples
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
using Moq;
using Bicep.Core.FileSystem;
using Bicep.Core.Configuration;
using Bicep.Core.UnitTests.Configuration;

namespace Bicep.Core.UnitTests.TypeSystem.Az
{
Expand Down
14 changes: 8 additions & 6 deletions src/Bicep.Core/Configuration/ConfigHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,26 @@ public class ConfigHelper
{
private const string bicepConfigResourceName = "Bicep.Core.Configuration.bicepconfig.json";
private readonly IFileResolver fileResolver;
private readonly bool useDefaultConfig;

/// <summary>
/// Property exposes the configuration root
/// that is currently loaded
/// </summary>
public IConfigurationRoot Config { get; private set; }

public ConfigHelper(string? localFolder, IFileResolver fileResolver)
public ConfigHelper(string? localFolder, IFileResolver fileResolver, bool useDefaultConfig = false)
{
this.fileResolver = fileResolver;
this.useDefaultConfig = useDefaultConfig;

if (localFolder is not null)
{
this.Config = BuildConfig(localFolder);
}
else
{
this.Config = BuildConfig(Directory.GetCurrentDirectory());
this.Config = BuildConfig(Directory.GetCurrentDirectory(), useDefaultConfig);
}
}

Expand All @@ -42,7 +44,7 @@ private IConfigurationRoot BuildConfig(Uri fileUri)
return BuildConfig(localFolder);
}

private IConfigurationRoot BuildConfig(string? localFolder)
private IConfigurationRoot BuildConfig(string? localFolder, bool useDefaultConfig = false)
{
var configBuilder = new ConfigurationBuilder();

Expand All @@ -56,7 +58,7 @@ private IConfigurationRoot BuildConfig(string? localFolder)
configBuilder.AddJsonStream(defaultConfigStream);

// last added json settings take precedent - add local settings last
if (DiscoverLocalConfigurationFile(localFolder) is string localConfig)
if (!useDefaultConfig && DiscoverLocalConfigurationFile(localFolder) is string localConfig)
{
// we must set reloadOnChange to false here - if it set to true, then ConfigurationBuilder will initialize
// a FileSystem.Watcher instance - which has severe performance impact on non-Windows OSes (https://github.com/dotnet/runtime/issues/42036)
Expand All @@ -82,7 +84,7 @@ private IConfigurationRoot BuildConfig(string? localFolder)
catch (Exception ex)
{
throw new Exception(
string.Format("Could not load configuration file. {0}", ex.InnerException?.Message ?? ex.Message));
string.Format("Could not load configuration file. {0} Please fix errors in the following configuration file: {1}", ex.InnerException?.Message ?? ex.Message, this.CustomSettingsFileName));
}
}
}
Expand Down Expand Up @@ -149,7 +151,7 @@ public T GetValue<T>(string settingPath, T defaultValue)
public ConfigHelper OverrideSetting(string name, object value)
{
SettingOverrides[name] = value;
this.Config = BuildConfig(Directory.GetCurrentDirectory());
this.Config = BuildConfig(Directory.GetCurrentDirectory(), useDefaultConfig);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,8 @@
// Licensed under the MIT License.

using Bicep.Core.Analyzers.Linter;
using Bicep.Core.Configuration;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Bicep.Core.UnitTests.Configuration
namespace Bicep.Core.Configuration
{
public static class ConfigHelperExtensions
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
using Bicep.Decompiler.Exceptions;
using Bicep.Decompiler;
using Bicep.Core.Configuration;
using Bicep.Core.UnitTests.Configuration;
using Bicep.Core.Registry;
using Bicep.Core.UnitTests;

Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.LangServer.IntegrationTests/BicepConfigTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ private async Task VerifyDiagnosticsAsync(MultipleMessageListener<PublishDiagnos
diagsParams.Diagnostics.Should().SatisfyRespectively(
x =>
{
x.Message.Should().Be(message);
x.Message.Should().Contain(message);
x.Severity.Should().Be(diagnosticSeverity);
x.Code?.String.Should().Be(code);
x.Range.Should().Be(new Range
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Bicep.Core.Samples;
using Bicep.Core.Semantics;
using Bicep.Core.UnitTests;
using Bicep.Core.UnitTests.Configuration;
using Bicep.Core.UnitTests.Utils;
using Bicep.Core.Workspaces;
using Bicep.LanguageServer.Providers;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void RefreshCompilationOfSourceFilesInWorkspace_WithInvalidBicepConfigFil
diagnostics.Should().SatisfyRespectively(
x =>
{
x.Message.Should().Be("Could not load configuration file. Expected depth to be zero at the end of the JSON payload. There is an open JSON object or array that should be closed. LineNumber: 8 | BytePositionInLine: 13.");
x.Message.Should().Contain("Could not load configuration file. Expected depth to be zero at the end of the JSON payload. There is an open JSON object or array that should be closed. LineNumber: 8 | BytePositionInLine: 13.");
x.Severity.Should().Be(DiagnosticSeverity.Error);
x.Code?.String.Should().Be("Fatal");
x.Range.Should().Be(new Range
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using Bicep.Core.TypeSystem;
using Bicep.Core.UnitTests;
using Bicep.Core.UnitTests.Assertions;
using Bicep.Core.UnitTests.Configuration;
using Bicep.LanguageServer.Completions;
using Bicep.LanguageServer.Snippets;
using FluentAssertions;
Expand Down

0 comments on commit 699af2a

Please sign in to comment.