DataSource draft #4112
DataSource draft #4112
Conversation
There was a problem hiding this comment.
For the name of the package, I was thinking about something like MSTest.Data.Csv or MSTest.DataSource.Csv or because we go for a functionally oriented name maybe the NuGet package could be tuned around the technology used so MSTest.Data.Oledb or MSTest.DataSource.Oledb. WDYT?
As for the name of the dll, my plan for v4 is to simplify naming so I think (if there is no technical constraint) we should do the same for these new packages.
src/TestFramework/TestFramework.Extensions.Csv/CsvDataSourceAttribute.cs
Outdated
Show resolved
Hide resolved
src/TestFramework/TestFramework.Extensions.Csv/CsvDataSourceAttribute.cs
Show resolved
Hide resolved
|
|
||
| <PropertyGroup> | ||
| <TargetFramework>netstandard2.0</TargetFramework> | ||
| <!-- TODO: (before merge) Is this hack relevant for a new project? --> |
There was a problem hiding this comment.
Good question, it's probably fine as this is (was?) mainly for some VS binding redirects.
There was a problem hiding this comment.
@Evangelink by fine, do you mean it can be removed for the new projects? or do you mean just keep as-is? I don't have much context on what issues is it trying to solve
src/TestFramework/TestFramework.Extensions.Csv/CsvDataSourceAttribute.cs
Outdated
Show resolved
Hide resolved
To me, the fact that we use OleDb for Csv is an implementation detail that can (and likely will) change in future. And for XML, we are not using OleDb at all and don't have extra dependencies. I think |
| using OleDbConnection connection = new(); | ||
|
|
||
| // We have to use the name of the folder which contains the CSV file in the connection string | ||
| // If target platform is x64, then use CsvConnectionTemplate64 connection string. |
|
|
||
| IEnumerable<object?[]> ITestDataSource.GetData(MethodInfo methodInfo) | ||
| { | ||
| // TODO: Avoid using OleDb and instead parse Csv directly. Maybe use https://www.nuget.org/packages/CsvHelper? Write our own? |
There was a problem hiding this comment.
I would definitely avoid writing our own. 😁
| @@ -52,6 +57,7 @@ public class DataSourceTests : AcceptanceTestBase | |||
|
|
|||
|
|
|||
| #file MyTestClass.cs | |||
| using System.Data; | |||
|
|
My view here is that if we wanted to replace OleDb with something else, it should be done in the same package, not as a new package. To me, the fact that we use OleDb is very implementation detail. Are you thinking of any scenarios where the user will still want to use OleDb with Csv if we provided an alternative? |
|
As described on the linked tickets, we have decided not to move forward with the feature request. |
Fixes #4092