Comments (10)
Agreed, but the previous code flow behaves this way so I feel like updating it would be out of scope for this change.
from home.
I only checked one test, Microsoft.NET.Build.Tests.GivenThatWeWantDiagnosticsWhenAssetsFileCannotBeRead.It_reports_corrupt_file, but its failure is:
System.NullReferenceException: Object reference not set to an instance of an object.
at NuGet.ProjectModel.LockFile.GetTarget(String frameworkAlias, String runtimeIdentifier)
at Microsoft.NET.Build.Tasks.LockFileExtensions.GetTargetAndReturnNullIfNotFound(LockFile lockFile, String frameworkAlias, String runtimeIdentifier) in D:\src\sdk\src\Tasks\Microsoft.NET.Build.Tasks\LockFileExtensions.cs:line 14
My understanding is that this means LockFileFormat
didn't not error out while loading the assets file, and now the SDK the is trying to pull information out of the LockFile
data structure (via the LockFile.GetTarget
method we provide), and it's throwing a null reference exception.
The test name makes it clear it's testing error scenarios, so it's not a "serious" failure (maybe the new code correctly handles all happy cases?), but the error handling appears to have changed with our STJ implementation.
from home.
NuGet/NuGet.Client#5637 fixed the It_reports_corrupt_file
test, now we just have the other one remaining:
Microsoft.NET.Build.Tasks.UnitTests.GivenAResolvePackageDependenciesTask.ItAssignsDiagnosticLevel [FAIL]
Expected value to be 3, but found 0 (difference of -3).
Stack Trace:
at FluentAssertions.Execution.XUnit2TestFramework.Throw(String message)
at FluentAssertions.Execution.TestFrameworkProvider.Throw(String message)
at FluentAssertions.Execution.DefaultAssertionStrategy.HandleFailure(String message)
at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc)
at FluentAssertions.Execution.AssertionScope.FailWith(Func`1 failReasonFunc)
at FluentAssertions.Execution.AssertionScope.FailWith(String message, Object[] args)
at FluentAssertions.Numeric.NumericAssertions`2.Be(T expected, String because, Object[] becauseArgs)
/_/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAResolvePackageDependenciesTask.cs(174,0): at Microsoft.NET.Build.Tasks.UnitTests.GivenAResolvePackageDependenciesTask.ItAssignsDiagnosticLevel()
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
from home.
@zivkan @jgonz120 I extracted the test into a standalone repro:
var lockFile = new NuGet.ProjectModel.LockFileFormat().Read("failing.lock.json");
with this file: failing.lock.json
It looks like it's failing to parse the file, e.g. Version is -2147483648
instead of the expected 3
from home.
@zivkan @jgonz120 I extracted the test into a standalone repro:
var lockFile = new NuGet.ProjectModel.LockFileFormat().Read("failing.lock.json");
with this file: failing.lock.json
It looks like it's failing to parse the file, e.g. Version is
-2147483648
instead of the expected3
The lock file contains a string for the warning level when it's supposed to be a number. The test needs to be updated to create a properly formatted file.
from home.
@jgonz120 shouldn't LockFileFormat
be throwing an format exception, rather than successfully returning an object with an invalid value? I don't think it's just a test setup problem.
from home.
I can confirm it parses when I change the warningLevel to be a number instead of a string so this should unblock the sdk code flow.
I agree though that throwing some exception during parsing would be better.
from home.
Interesting. That also means code like this doesn't make sense since we'll never get an exception: https://github.com/dotnet/sdk/blob/302ca4c93537954c056289cde3a2c4faac0d9ce2/src/Cli/Microsoft.DotNet.Cli.Utils/Extensions/LockFileFormatExtensions.cs#L25
from home.
Yea seems like it. In the LockFileCache class there's a logger that throws an error when LogError is called, something like that could be used to make that code make sense. https://github.com/dotnet/sdk/blob/302ca4c93537954c056289cde3a2c4faac0d9ce2/src/Tasks/Microsoft.NET.Build.Tasks/LockFileCache.cs#L79
from home.
Actually I take that back, this calls public LockFile Read(string filePath)
which uses File.OpenRead(filePath)
before passing the stream to the other Read method so we'd catch the exception from there.
I think we can close this issue.
from home.
Related Issues (20)
- Duplicate sources can lead to unnecessary assets file writes.
- Using the 'TargetFrameworks' and 'OutputPath' causes command 'dotnet pack --artifacts-path' to be abnormal HOT 1
- prop/target files in packages are not added correctly for C++/CLI projects. HOT 5
- [Dotnet Package Search] The package sorting from “dotnet package search” is different with PM UI HOT 3
- [Dotnet Package Search] The column “Owners” should not be empty when running the command “dotnet package search <Package Name> --exact-match” HOT 3
- [Dotnet Package Search] The search result is hard to read when searching with detailed verbosity HOT 3
- [Dotnet Package Search] An unhandled exception is thrown when searching with “--verbosity detailed” and “--format json”
- Respect DOTNET_NUGET_CONFIG and GLOBAL_JSON environment variables HOT 9
- Plugin issue while using any of the nuget command HOT 3
- PrivateAssets=all has different behavior than PrivateAssets=compile;build;buildTransitive.... HOT 3
- When Version differs on a per framework level in nomination, indicate to the customer that PackageVersion can be used to resolve the inconsistency HOT 1
- [Bug Bash] The vulnerability InfoBar does not display in the Solution Explorer window of “packages.config” project until restoring at the second time
- [Bug Bash] The Error List window only displays the vulnerability warning for the first installed vulnerable package
- dotnet list package: fails when using <PROJECT | SOLUTION> argument HOT 6
- dotnet list <SOLUTION> package --vulnerable/--outdated/--deprecated fails with Azure Artifacts Credential Provider HOT 5
- Roll-out new breaking change process for SDK tools, respect SdkAnalysisLevel HOT 1
- Missing test coverage on `ConcurrencyUtilities.ExecuteWithFileLockedAsync`
- "Duplicate frameworks found" when restoring using static graph HOT 5
- `MSBuildRestoreUtility.GetRestoreAuditProperties` needs a breaking change to read `NuGetAuditSuppress` items
- [Dotnet Package Search] Search result doesn’t ignore empty property “owners” in JSON format output for offline package source “C:\Program Files (x86)\Microsoft SDKs\NuGetPackages” HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from home.