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

Test case improvements and updates #2393

Merged
merged 21 commits into from
Nov 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c40073b
TestHalf2 and TestHalf now can pass in suite
dloe Jul 2, 2024
d9dc31a
Added similar prints for Half3 and Half4, added comments on Converter
dloe Jul 2, 2024
46ad9cb
LifeTimeNoSimpleConstructor now passes
dloe Jul 3, 2024
6162014
RenderToWindow test now skips due to failure
dloe Jul 3, 2024
dc984b5
Various Graphics Tests Using outdated images [DONT USE]
dloe Jul 3, 2024
ac0a968
TEstGCHandleAlloc: Autolayout was set to fail when it does now in fac…
dloe Jul 5, 2024
54e0570
Stride Particles Tests Updated Visual Tests
dloe Jul 5, 2024
125d491
Updated TestGetFontInfo to now properly check if various Font Info is…
dloe Jul 15, 2024
d598082
Added skips for graphics related tests to avoid confusion and give ad…
dloe Jul 15, 2024
51c887b
Updated template package names to now be properly loaded
dloe Jul 16, 2024
b29156d
Revert "Stride Particles Tests Updated Visual Tests"
dloe Jul 16, 2024
8995c41
Revert "Various Graphics Tests Using outdated images [DONT USE]"
dloe Jul 16, 2024
110a916
Added skips to GPU rendering related tests that will consecutively fail
dloe Jul 17, 2024
a8916ca
Merge branch 'master' into Test-Case-Improvements-and-updates
dloe Jul 17, 2024
5ccd93b
Skipped 2 more rendering related test cases, cleaned comments
dloe Jul 17, 2024
87c7bda
Revert "Added skips to GPU rendering related tests that will consecut…
dloe Jul 18, 2024
ac882ba
Revert "Added skips for graphics related tests to avoid confusion and…
dloe Jul 18, 2024
ee50b87
Removed changed tests back to original tests with more comments
dloe Jul 18, 2024
c488249
Revert "Updated template package names to now be properly loaded"
dloe Jul 18, 2024
bcd5d06
Removed Skips for rendering
dloe Jul 18, 2024
d2d3496
Removed LifetimeNoSimpleConstuctor test and unnecessary constructor
dloe Jul 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions sources/core/Stride.Core.Design.Tests/TestTypeConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,25 +41,25 @@ public void TestColor4()
TestConversionMultipleCultures(new Color4(0.25f, 50.0f, -4.9f, 1));
}

[Fact(Skip = "fix Half.ToString() and update converter accordingly (to match other converters)")]
[Fact]
public void TestHalf()
{
TestConversionMultipleCultures(new Half(5.6f));
}

[Fact(Skip = "fix Half2.ToString() and update converter accordingly (to match other converters)")]
[Fact]
public void TestHalf2()
{
TestConversionMultipleCultures(new Half2(new Half(5.12f), new Half(2)));
}

[Fact(Skip = "fix Half3.ToString() and update converter accordingly (to match other converters)")]
[Fact]
public void TestHalf3()
{
TestConversionMultipleCultures(new Half3(new Half(5.12f), new Half(2), new Half(-17.54f)));
}

[Fact(Skip = "fix Half4.ToString() and update converter accordingly (to match other converters)")]
[Fact]
public void TestHalf4()
{
TestConversionMultipleCultures(new Half4(new Half(5.12f), new Half(2), new Half(-17.54f), new Half(-5)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ protected static T[] ConvertToValues<T>(ITypeDescriptorContext context, CultureI
var converter = TypeDescriptor.GetConverter(typeof(T));
var strings = str.Trim().Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);

// Note: we explicitely box the struct so we can use reflection to set values.
// Note: we explicitly box the struct so we can use reflection to set values.
object result = new TResult();
foreach (var comp in strings)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public override bool CanConvertTo(ITypeDescriptorContext context, Type destinati
return true;
}

//TODO: Verify that converter is properly updated to match other converters
/// <summary>
/// Converts the given object to the type of this converter, using the specified context and culture information.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public override bool CanConvertTo(ITypeDescriptorContext context, Type destinati
return true;
}

//TODO: Verify that converter is properly updated to match other converters
/// <summary>
/// Converts the given object to the type of this converter, using the specified context and culture information.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public override bool CanConvertTo(ITypeDescriptorContext context, Type destinati
return true;
}

//TODO: Verify that converter is properly updated to match other converters
/// <summary>
/// Converts the given object to the type of this converter, using the specified context and culture information.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ public override bool CanConvertTo(ITypeDescriptorContext context, Type destinati
return true;
}

//TODO: Verify that converter is properly updated to match other converters
/// <summary>
/// Converts the given object to the type of this converter, using the specified context and culture information.
/// </summary>
Expand Down
14 changes: 14 additions & 0 deletions sources/core/Stride.Core.Mathematics/Half2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@

using System;
using System.ComponentModel;
using System.Globalization;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text.RegularExpressions;
using Stride.Core.Serialization;

namespace Stride.Core.Mathematics
Expand Down Expand Up @@ -158,6 +160,18 @@ public Half2(float value)
return !Equals(ref left, ref right);
}

/// <summary>
/// Returns a <see cref="string"/> that represents this instance.
/// </summary>
/// <returns>
/// A <see cref="string"/> that represents this instance.
/// </returns>
public override string ToString()
{
CultureInfo culture = CultureInfo.CurrentCulture;
return string.Format(culture, "{0}{1} {2}", X.ToString(), culture.TextInfo.ListSeparator, Y.ToString());
}

/// <summary>
/// Returns the hash code for this instance.
/// </summary>
Expand Down
13 changes: 13 additions & 0 deletions sources/core/Stride.Core.Mathematics/Half3.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

using System;
using System.ComponentModel;
using System.Globalization;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using Stride.Core.Serialization;
Expand Down Expand Up @@ -175,6 +176,18 @@ public Half3(float value)
return !Equals(ref left, ref right);
}

/// <summary>
/// Returns a <see cref="string"/> that represents this instance.
/// </summary>
/// <returns>
/// A <see cref="string"/> that represents this instance.
/// </returns>
public override string ToString()
{
CultureInfo culture = CultureInfo.CurrentCulture;
return string.Format(culture, "{0}{1} {2}{1} {3}", X.ToString(), culture.TextInfo.ListSeparator, Y.ToString(), Z.ToString());
}

/// <summary>
/// Returns the hash code for this instance.
/// </summary>
Expand Down
13 changes: 13 additions & 0 deletions sources/core/Stride.Core.Mathematics/Half4.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

using System;
using System.ComponentModel;
using System.Globalization;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using Stride.Core.Serialization;
Expand Down Expand Up @@ -192,6 +193,18 @@ public Half4(float value)
return !Equals(ref left, ref right);
}

/// <summary>
/// Returns a <see cref="string"/> that represents this instance.
/// </summary>
/// <returns>
/// A <see cref="string"/> that represents this instance.
/// </returns>
public override string ToString()
{
CultureInfo culture = CultureInfo.CurrentCulture;
return string.Format(culture, "{0}{1} {2}{1} {3}{1} {4}", X.ToString(), culture.TextInfo.ListSeparator, Y.ToString(), Z.ToString(), W.ToString());
}

/// <summary>
/// Returns the hash code for this instance.
/// </summary>
Expand Down
21 changes: 0 additions & 21 deletions sources/core/Stride.Core.Tests/TestContentManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -284,27 +284,6 @@ public void LifetimeShared()
Assert.Equal(0, ((IReferencable)c1ChildCopy).ReferenceCount);
}

[Fact(Skip = "Need check")]
public void LifetimeNoSimpleConstructor()
{
var c1 = new C { I = 18 };
c1.Child2 = new D(18);

var databaseProvider = CreateDatabaseProvider();
var assetManager1 = new ContentManager(databaseProvider);
var assetManager2 = new ContentManager(databaseProvider);

assetManager1.Save("c1", c1);

var c1Copy = assetManager2.Load<C>("c1");
Assert.Equal(1, ((IReferencable)c1Copy).ReferenceCount);
Assert.Equal(1, ((IReferencable)c1Copy.Child2).ReferenceCount);

assetManager2.Unload(c1Copy);
Assert.Equal(0, ((IReferencable)c1Copy).ReferenceCount);
Assert.Equal(0, ((IReferencable)c1Copy.Child2).ReferenceCount);
}

[Fact]
public void LifetimeCycles()
{
Expand Down
3 changes: 2 additions & 1 deletion sources/engine/Stride.Engine.Tests/TestUpdateEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class TestUpdateEngine
[StructLayout(LayoutKind.Sequential)] public struct SequentialLayoutObjRef { public object Ref; }
[StructLayout(LayoutKind.Explicit)] public struct ExplicitLayoutObjRef { [FieldOffset(0)] public object Ref; }


[Fact]
public unsafe void TestGCHandleAlloc()
{
Expand All @@ -33,7 +34,7 @@ static bool CanPin<T>()
return false;
}
}
Assert.False(CanPin<AutoLayout>());
Assert.True(CanPin<AutoLayout>());
Comment on lines -36 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's ask the author about this, hopefully he has time to get back to us regarding this line.
@ericwj do you remember why you introduced this specific assertion ? As far as I can tell empty autolayouts have always been pinnable ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh you pinged me a while ago. I must've completely missed the mention. Sorry about that @Eideren.

I'm pretty sure if it was part of the PR and got accepted, the test passed. And indeed it does on .NET Framework, which is the easiest for me to test without installing old .NET runtimes. It is very possible that it also passes on older .NET Core or .NET 5/6. But if it now doesn't pass and you adjusted it to that circumstance, it'll likely be fine.

It is testing behavior of the runtime and that appears to have changed sometime. It was convenient that this test returned false just to know that in some places nobody forgot to change struct layout from the default of auto, at least where pinning was attempted - I believe this could be the case in the animation infrastructure. I don't think there is any other impact of this change in behavior.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Coincidentally I come accross another situation where a test crashes and because of this here, I checked and it happens that it is 2024 when I learn that already on .NET 2.0 an unannotated struct is in fact layout sequential. I must've seen this so many times in ILSpy or wherever... Not knowledge active enough to not surprise me. Somehow I assumed it to be auto, probably when writing the above code. At least when I wrote the reply...

Assert.True(CanPin<SequentialLayout>());
Assert.True(CanPin<ExplicitLayout>());
Assert.False(CanPin<AutoLayoutObjRef>());
Expand Down
11 changes: 7 additions & 4 deletions sources/engine/Stride.Graphics.Tests/TestFontManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,20 @@ public void TestDoesFontContains()
fontManager.Dispose();
}

//Note: Test may fail due to some issues with SharpFont.
//Updated TestGetFontInfo to now properly check if various Font Info is properly loaded
[Fact]
public void TestGetFontInfo()
{
Init();

var fontManager = new FontManager(CreateDatabaseProvider());

float lineSpacing = 0;
float baseLine = 0;
float width = 0;
float height = 0;
float lineSpacing = 0f;
float baseLine = 0f;
float width = 0f;
float height = 0f;

fontManager.GetFontInfo("Risaltyp_024", FontStyle.Regular, out lineSpacing, out baseLine, out width, out height);
Assert.Equal(4444f / 4096f, lineSpacing);
Assert.Equal(3233f / 4096f, baseLine);
Expand Down