aplteam / Cider

Cider is a Project Manager for Dyalog APL
MIT License
10 stars 5 forks source link

Improve the NuGet-functionality #60

Open aplteam opened 7 months ago

aplteam commented 7 months ago

That increases the list of changed files - we don't want that!

Those tests should copy the test data to a temp folder, and that's what the tests in question should use. Afterwards the tests should remove that folder, leaving no footprint.

Alexander-Block commented 7 months ago

I am not sure whether this is literally true or a misunderstanding. I assume you are refering to the test cases Test_NuGet_010 and Test_UC_045 for which a lot of "transient" data is removed in the current state of PR #59.

However, these irrelevant pieces of data are not the result of the two test cases being executed in the project folder themselves. Both tests are bases on Cider projects with installed NuGet dependencies. For instance, the test case description for Test_NuGet_010 reads:

⍝ Open a project that has already two NuGet dependencies

The current approach to test this (before PR #59) is to have a fully configured Cider project (with NuGet dependencies) under the path Tests/Test_NuGet_010 that is copied to a temporary folder and opened in Cider from there. The problem is that the Cider project under Tests/Test_NuGet_010 has been generated by executing ]Cider.AddNuGetDependencies beforehand. This approach has two problems:

  1. A fully configured Cider project with NuGet dependencies has these dependencies in a publication folder which contains dlls targeting a specific target framework. A Cider project build with net5.0 as target will not be compatible with a Dyalog APL installation targeting net6.0. This is the reason why in PR #59 I split the test data for Test_NuGet_010 in two subfolders Tests/Test_NuGet_010/net5.0 and Tests/Test_NuGet_010/net6.0 that only vary by the target framework.
  2. When using NuGet.Add we ultimately call dotnet publish. This command is meant to create a self-contained folder published containing all assemblies and metadata to deploy a dotnet application anywhere. However, in order to do this, the dotnet application which covers our NuGet dependencies has to be built first. The bin and obj folders will contain the build artifacts as well as metadata that is specific to the environment in which the build took place. This is the reason that we saw file paths that only make sense in the context of my machine in the first version of #59.

With respect to the tests we can get rid of both problems by removing the hard-coded Cider projects and changing the tests to first create Cider projects with the desired specifications (under a temporary folder, of course) and then exercising it. However, this tests not quite the same thing and in fact for Test_NuGet_010 this would be equivalent to deleting this test, since Test_NuGet_004 is essentially the test we would get by modifying Test_NuGet_010 in this way.

However, I think that the observations 1 and 2 above transcend test:

  1. I think a Cider project should not depend on a target framework when this is not strictly necessary. This is probably hard to acommodate.
  2. We should probably not store transient data in a Cider project at all. I think it would make sense to automatically delete the bin and obj folders after adding a NuGet dependency via ]Cider.AddNuGetDependencies. That is essentially what I manually did in my last change to PR #59.