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

Fully support IsTestProject #736

Closed
wants to merge 0 commits into from

Conversation

AlbertoMonteiro
Copy link
Contributor

This PRs aims to fully support IsTestProject.

I am using Buildalyzer to get this information.

This is a draft PR, before finishing this PR I would like if is there any impeditive with this solution.

Fixes #669

@sonatype-lift
Copy link

sonatype-lift bot commented Jul 18, 2023

Sonatype Lift is retiring

Sonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
We are extremely grateful and thank you for your support over the years.

📖 Read about the impacts and timeline

@Bertk
Copy link
Contributor

Bertk commented Jul 18, 2023

The new package Buildalyzer generates the necessary values with a design-time build. 👍

image

Microsoft.Build package version 17.3.2 supports net6.0 and later versions support net7.0. This needs to be checked for the CycloneDX tools package generation which supports both.

  • Now, with this new component some implementation which is using XmlReader could be replaced as well.
  • the Buildalyzer with dependencies adds 4 MB to CycloneDX tools packages
image

@AlbertoMonteiro
Copy link
Contributor Author

@Bertk I've tried it locally targeting a solution with .NET 7, everything worked without any issues.

I am going to test with other projects, n6, n7 and multi target n6/7 aswell.

I will keep updating this PR, thanks for quick answer.

@Bertk
Copy link
Contributor

Bertk commented Jul 19, 2023

@AlbertoMonteiro Buildalyzer comes with a huge transitive dependency list but adds some missing MSBuild functionality.
@coderpatros Please send your feedback for this PR.

dotnet list package --include-transitive --source https://api.nuget.org/v3/index.json

I updated some package versions and this outdated dependencies are remaining (CycloneDX nuget package size 9,29 MB)
image

<Project>
  <PropertyGroup>
    <CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
  </PropertyGroup>

  <ItemGroup>
    <PackageVersion Include="CycloneDX.Core" Version="5.4.0" />
    <PackageVersion Include="McMaster.Extensions.CommandLineUtils" Version="4.0.2" />
    <PackageVersion Include="Microsoft.DotNet.PlatformAbstractions" Version="3.1.6" />
    <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.5.0" />
    <PackageVersion Include="NuGet.ProjectModel" Version="6.6.1" />
    <PackageVersion Include="NuGet.Protocol" Version="6.6.1" />

    <PackageVersion Include="Moq" Version="4.18.4" />

    <PackageVersion Include="RichardSzalay.MockHttp" Version="6.0.0" />

    <PackageVersion Include="xunit" Version="2.5.0" />
    <PackageVersion Include="xunit.runner.visualstudio" Version="2.5.0" />

    <PackageVersion Include="coverlet.collector" Version="6.0.0" />
    <PackageVersion Include="coverlet.msbuild" Version="6.0.0" />

    <PackageVersion Include="System.IO.Abstractions" Version="19.2.26" />
    <PackageVersion Include="System.IO.Abstractions.TestingHelpers" Version="19.2.26" />

    <PackageVersion Include="Buildalyzer" Version="5.0.0" />

    <Packageversion Include="JsonSchema.Net" Version="4.1.6" />
    <Packageversion Include="JetBrains.Annotations" Version="2023.2.0" />
    <Packageversion Include="Microsoft.Build" Version="17.3.2" />
    <Packageversion Include="Microsoft.Build.Framework" Version="17.3.2" />
    <Packageversion Include="Microsoft.Build.Tasks.Core" Version="17.3.2" />
    <Packageversion Include="Microsoft.Build.Utilities.Core" Version="17.3.2" />
    <Packageversion Include="Microsoft.Extensions.Configuration" Version="7.0.0" />
    <Packageversion Include="Microsoft.Extensions.Configuration.Abstractions" Version="7.0.0" />
    <Packageversion Include="Microsoft.Extensions.Configuration.Binder" Version="7.0.4" />
    <Packageversion Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="7.0.0" />
    <Packageversion Include="Microsoft.Extensions.DependencyModel" Version="7.0.0" />
    <Packageversion Include="Microsoft.Extensions.Logging" Version="7.0.0" />
    <Packageversion Include="Microsoft.Extensions.Logging.Abstractions" Version="7.0.1" />
    <Packageversion Include="Microsoft.Extensions.Options" Version="7.0.1" />
    <Packageversion Include="Microsoft.Extensions.Primitives" Version="7.0.0" />
    <Packageversion Include="Microsoft.NET.StringTools"  Version="17.6.3" />
    <Packageversion Include="Microsoft.NETCore.Platforms" Version="7.0.4" />
    <Packageversion Include="Microsoft.NETCore.Targets"  Version="5.0.0" />
    <Packageversion Include="MSBuild.StructuredLogger"  Version="2.1.815" />
    <Packageversion Include="Newtonsoft.Json"  Version="13.0.3" />
    <Packageversion Include="protobuf-net"  Version="3.2.26" />
    <Packageversion Include="TestableIO.System.IO.Abstractions"  Version="19.2.29" />
    <Packageversion Include="TestableIO.System.IO.Abstractions.Wrappers"  Version="19.2.29" />

    <Packageversion Include="System.Buffers" Version="4.5.1" />
    <Packageversion Include="System.CodeDom" Version="7.0.0" />
    <Packageversion Include="System.Console" Version="4.3.1" />
    <Packageversion Include="System.Diagnostics.DiagnosticSource" Version="7.0.2" />
    <Packageversion Include="System.Diagnostics.EventLog" Version="7.0.0" />
    <Packageversion Include="System.Drawing.Common" Version="7.0.0" />
    
  </ItemGroup>
</Project>

@AlbertoMonteiro AlbertoMonteiro marked this pull request as ready for review July 19, 2023 20:06
@AlbertoMonteiro AlbertoMonteiro changed the title WIP: Fully support IsTestProject Fully support IsTestProject Jul 19, 2023
@AlbertoMonteiro
Copy link
Contributor Author

@Bertk I've finished it.

I have tested with net6 and net7 projects, it worked normally.

I also had to create real test projects because the AnalyzerManager needs real file system access, so I replicated one of the test scenarios where I really needed it.

@Bertk
Copy link
Contributor

Bertk commented Jul 20, 2023

@AlbertoMonteiro The CI was cancelled after 30 minutes. Please check.

I try to analyze the buildanlyzer project and found a extraordinary test strategy e.g. using other github repositories as test object. Until now I could not run all buildanlyzer tests using my desktop system and there is no documentation for a development environment 😏 - this raises some questions.

@AlbertoMonteiro
Copy link
Contributor Author

@AlbertoMonteiro The CI was cancelled after 30 minutes. Please check.

@Bertk this is fixed

I try to analyze the buildanlyzer project and found a extraordinary test strategy e.g. using other github repositories as test object. Until now I could not run all buildanlyzer tests using my desktop system and there is no documentation for a development environment 😏 - this raises some questions.

About this other repos as test object, can you explain me?

@Bertk
Copy link
Contributor

Bertk commented Jul 21, 2023

@AlbertoMonteiro buildanlyzer project has unit and integration tests. Typically functionality is verified and a test should cover a specific implementation topic. buildanlyzer project integration test are different and checks integration using source code from 2 GitHub repositories. I am not sure whether this is a good choice ...

@AlbertoMonteiro
Copy link
Contributor Author

@Bertk yeah,i saw that they use git submodules, I was able to run their tests after installing .NET Full Framework 4.6
2 sdk.

For the tests that there is in cyclone I think having the dummy project to he used in tests is fine.

Copy link
Contributor

@Bertk Bertk left a comment

Choose a reason for hiding this comment

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

👍 Please contact @coderpatros for review and approval.

@Bertk
Copy link
Contributor

Bertk commented Jan 8, 2024

@AlbertoMonteiro Please rebase the branch and resolve the conflicts. There is also a new version of Buildalyzer available. This enhancement should be added ASAP😃

@mtsfoni Please review and merge the PR. This will be the foundation for the solution for #699 (determine MSBuild property values)

@AlbertoMonteiro
Copy link
Contributor Author

@Bertk done!

@mtsfoni
Copy link
Contributor

mtsfoni commented Jan 8, 2024

I want to look at a few open bugs first. After that, this could become our 3.1.0 - together with #699, maybe.

@Bertk
Copy link
Contributor

Bertk commented Jan 11, 2024

@AlbertoMonteiro Hi, sorry to bother you again but there was another release and now we see again branch conflicts.

Could you please fix the merge conflicts.

@mtsfoni
Copy link
Contributor

mtsfoni commented Jan 14, 2024

Calling project.Build() deletes many file in the bin directory. Normally a pipeline would be done with those files when creating a SBOM, but I've seen people do all kinds of weird things.

Is it possible to redirect the BuildAnalyzer "BuildTarget" to some temporary folder, so that CycloneDX does not mess with the Build outputs?

@Bertk
Copy link
Contributor

Bertk commented Jan 15, 2024

Is it possible to redirect the BuildAnalyzer "BuildTarget" to some temporary folder, so that CycloneDX does not mess with the Build outputs?

The .NET 8.0 SDK Artifacts output layout can be used to separate the Buildalyzer artifacts e.g.

       var manager = new AnalyzerManager();
       manager.SetGlobalProperty("UseArtifactsOutput", "true");
       manager.SetGlobalProperty("ArtifactsPath", Path.GetFullPath(Path.Combine(projectFilePath, "..\\..\\.alyzertmp")));
image

@mtsfoni
Copy link
Contributor

mtsfoni commented Jan 18, 2024

I'll try to integrate this pull request this weekend.

I want to do some changes like adding it as a service-class as we will certainly add more functions that will access it. Also caching the results for the same reason.

@AlbertoMonteiro Thank you for your groundwork. You don't need to rebase again.

@mtsfoni mtsfoni mentioned this pull request Jan 21, 2024
@mtsfoni
Copy link
Contributor

mtsfoni commented Jan 21, 2024

I made a mistake integrating my changes and by that I caused the PR to be closed, which took away permission from me to interact with it.

I reopened as #837

@AlbertoMonteiro if you care for those GitHub points and want a merged PR, we can find some small change you could request that I can immediately merge.

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.

Test projects are not excluded with option "exclude-test-projects"
3 participants