-
-
Notifications
You must be signed in to change notification settings - Fork 3
Add unit tests for hashtable with classes as keys #123
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
Conversation
dc3f0c6
to
29cee65
Compare
Maybe the test doesn't produce the type as a class which it fails on with the WPF example |
I tried the code on Real Hardware and it failed with a system crash. It crashed on the eight iteration of adding into HashTable. Debug Trace
|
a439c79
to
fc892eb
Compare
fc892eb
to
2e77991
Compare
Kudos, SonarCloud Quality Gate passed!
|
Note Investigation run on an ESP32 with PSRAM with ESP32_PSRAM_REV0-1.9.0.118 firmware. I've been playing a bit with all this and here is the finding:
Here is the simple code: using System;
using System.Collections;
AnotherTest();
void AnotherTest()
{
// This is what we will adjust
var hashtable = new Hashtable(1000);
Console.WriteLine("Adding 100 elements to the hashtable with SomeOtherKey class");
for (int i = 0; i < 100; i++)
{
SomeOtherKey keyt = new SomeOtherKey(i);
hashtable.Add(keyt, keyt.I.GetHashCode());
}
foreach (var keyo in hashtable.Keys)
{
if ((keyo as SomeOtherKey).I.GetHashCode() != (int)hashtable[keyo])
{
Console.WriteLine($"Failed on the {keyo} key.");
}
}
} This fails at 25th element! If you remove the 1000 from the initialisation of the Hashtable, this now fails at 4! With initialization of 5000, 67th OK, now you got the math so But what if you have 150 elements? Well, it will then always fails because |
Can I confirm that if the Class has Constructor then you cannot directly get the GetHashCode and add into the HashTable? Code Works
Code Fails
|
@alberk8 did you run this before or after the fix was merged? I just run it from an STM32F769 DISCO and ESP_WROVER_KIT fw 1.9.0.118 without any issues... key with or without constructor, getting hash from it, etc |
@josesimoes , I tried again on ESP32_REV3 v1.9.0.901 and it crash on the the third iteration. Error
Code
|
Interesting fact following several tests on multiple platforms:
🤔 |
@josesimoes , I added Could it be the HashCode is not ready when it is trying to add into the hashtable. |
@alberk8 root cause found and fixed. Thank you for the prompt reply as usual. 👍🏻 |
This should be fixed now with nanoframework/nf-interpreter#2838 |
I can happily report that with ESP32_PSRAM_REV0-1.9.0.122.zip the sample code is working. But I found another issue.Adjust the SomeOtherKey class like this: public class SomeOtherKey
{
// with or without the private i, the behavior is the same
//private readonly int _i;
public int I { get; private set; }//=> _i;
public int J { get; private set; }//=> _i;
//public SomeOtherKey()
//{
//}
public SomeOtherKey(int i)
{
//_i = i;
I = i;
}
} and then I get a "Resolving" and it does not even do the first Add properly.
|
@josesimoes , Testing with the latest ESP32_REV3 v1.9.0.908 firmware with the test code above runs but the hashtable key-value does not match.
Update |
@alberk8 confirmed. This should be an issue with the hashcode generation, something failing for classes. With this slight variation: public static void Hashtable_ClassAsKey()
{
var hashtable = new Hashtable();
for (int i = 0; i < 10; i++)
{
SomeKey key = new SomeKey(i);
hashtable.Add(key, i.GetHashCode());
Debug.WriteLine($"Added {i} -> {i.GetHashCode()} element.");
}
Debug.WriteLine($"Added 10 elements.");
foreach (SomeKey key in hashtable.Keys)
{
Debug.WriteLine($"{(key as SomeKey).GetValue.GetHashCode()} -> {(int)hashtable[key]} -> {(key as SomeKey).GetValue}");
}
} I get this:
When using a class:
|
@josesimoes Testing the latest firmware ESP32_REV3 v1.9.0.910 has another issue. The class below GetHashCode returns Zero. public class SomeKey |
@alberk8 say again please.... |
@josesimoes , Sorry the above class works. But If I have more than one property then the HashCode for the class is Zero. Debug Output
Code
|
Having no more feedback about this and considering the Unit Tests are now 🟢 after the fixes in native code, I'm merging this. |
Description
Motivation and Context
How Has This Been Tested?
Screenshots
Types of changes
Checklist: