Skip to content

Conversation

@nohwnd
Copy link
Member

@nohwnd nohwnd commented Mar 10, 2022

Description

Run acceptance tests against multiple versions of our components.

nohwnd and others added 30 commits March 4, 2022 11:47
… it is used with multiple different meanings
// Do not generate the data rows here, properties (e.g. DebugVSTestConsole) are not populated until after constructor is done.
}

public bool DebugVSTestConsole { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this will bring much but I see this set of properties appears in multiple data sources, is it worth having an intermediate type? If we don't foresee much change going forward I don't mind the "copy/paste" state.

Copy link
Member Author

Choose a reason for hiding this comment

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

How would that work?

Keep in mind that these are properties on attribute types, and they need to be from the set of valid attribute types. Bool is valid, but custom object is not because it is not a constant.

Copy link
Member

@Evangelink Evangelink Mar 24, 2022

Choose a reason for hiding this comment

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

I was thinking about inheritance and not composition here.

public abstract class TestDataSourceBase : TestDataSource<RunnerInfo>
{
	public bool DebugVSTestConsole { get; set; }
    public bool DebugTestHost { get; set; }
    public bool DebugDataCollector { get; set; }
    public bool NoDefaultBreakpoints { get; set; } = true;
}

public class MSTestCompatibilityDataSource : TestDataSourceBase
{
...
}

But you are right that we would need to do something similar for attribute so that's not ideal.

Copy link
Member

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

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

There are a couple of classes where you replaced some calls to the base TempDirectory for a local one. Is it on purpose or was it just for test and this needs to be reverted?

@nohwnd
Copy link
Member Author

nohwnd commented Mar 24, 2022

I looked at the PR and I could not find which classes are they? Can you give me an example?
I feel like I made a merge on some clone of the repo, but then did another merge on another clone of the repo, and pushed from there or something, because I do a lot of changes I thought I already did.

@Evangelink
Copy link
Member

They are all in ExecutionTests (12 instances).

@nohwnd
Copy link
Member Author

nohwnd commented Mar 24, 2022

Reverted that back to use TempDirectory from the base class.

@nohwnd nohwnd merged commit f506d43 into microsoft:main Mar 29, 2022
@nohwnd nohwnd deleted the test-matrix branch March 29, 2022 13:09
@nohwnd
Copy link
Member Author

nohwnd commented Mar 29, 2022

This was blocking on the DiscoveryCancellation test, that I stabilized, but broke by the threshold. I am fixing that in #3529

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.

2 participants