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

Refactor the test code #35

Closed
wants to merge 33 commits into from
Closed

Refactor the test code #35

wants to merge 33 commits into from

Conversation

rstm-sf
Copy link

@rstm-sf rstm-sf commented Oct 18, 2020

Hello!

Now that the tests are almost restored (#31), we can try refactoring the test code. This PR suggests doing it:

  • removes visibility for VS and dotnet test tests with netstandard2.0
  • separates classes for tests from the tests themselves

My primary goal was to try and get rid of the hacks for running tests with VS, but that didn't help

Depends on #31

Closes #33

{
public class PropertyTestConverter : TypeConverter
{
public static event Action<CultureInfo, ITypeDescriptorContext> ConvertFromEventRequiredAssert;
Copy link
Author

Choose a reason for hiding this comment

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

I have not found a better way how it would be possible to pass values ​​for validation

(int) ((ExtensionValueHolder) sp.GetService(typeof(ExtensionValueHolder))).Value;
}

public class CustomConvertedType
Copy link
Author

Choose a reason for hiding this comment

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

It looks like this class is not used anywhere

Comment on lines 33 to 41
- name: Test for net47 CecilNetstandardTests
if: matrix.os == 'windows-latest'
run: dotnet test --framework net47 tests/CecilNetstandardTests/CecilNetstandardTests.csproj
- name: Test for net47 CecilTests
if: matrix.os == 'windows-latest'
run: dotnet test --framework net47 tests/CecilTests/CecilTests.csproj
- name: Test for net47 XamlParserTests
if: matrix.os == 'windows-latest'
run: dotnet test --framework net47 tests/XamlParserTests/XamlParserTests.csproj
Copy link

@worldbeater worldbeater Oct 19, 2020

Choose a reason for hiding this comment

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

Consider checking out the following GitHub Actions setup:
https://github.com/reactiveui/ReactiveUI.Validation/blob/e7bc86ded6f6eeecc781094ad30283bff68fc5fc/.github/workflows/ci-build.yml#L48-L57

E.g. if you build the project first, you can then run unit tests for all built assemblies ending with Tests.csproj using the *Tests.csproj glob pattern and the --no-build option. This will run the tests for both netcoreapp3.1 and net47 as well, just in one command. Could be useful in removing all these if statements and in generating only one report for all the test projects instead.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, that looks interesting, thanks!

@worldbeater worldbeater mentioned this pull request Oct 19, 2020
4 tasks
@rstm-sf
Copy link
Author

rstm-sf commented Oct 21, 2020

Now by calling dotnet test we get the passed tests https://github.com/rstm-sf/XamlX/actions/runs/319622697

Copy link

@worldbeater worldbeater left a comment

Choose a reason for hiding this comment

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

Thanks @rstm-sf for doing this! The CI configuration looks really good so far.

@rstm-sf
Copy link
Author

rstm-sf commented Oct 23, 2020

@rstm-sf
Copy link
Author

rstm-sf commented Jul 13, 2023

@kekekeks hello, does it make sense to update it?

@rstm-sf rstm-sf closed this Sep 1, 2023
@rstm-sf rstm-sf deleted the refactor/extract_netstandard_tests branch September 1, 2023 10:38
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.

Tests/benchmarks failing
3 participants