nanoframework / Home

:house: The landing page for .NET nanoFramework repositories.
https://www.nanoframework.net
MIT License
860 stars 79 forks source link

StringBuilder requires extra unit tests #998

Closed networkfusion closed 2 years ago

networkfusion commented 2 years ago

Library/API/IoT binding

System.Text

Visual Studio version

VS2022

.NET nanoFramework extension version

latest

Target name(s)

OrgPalThree

Firmware version

1.7.4.92

Device capabilities

** .NET nanoFramework extension v2022.1.0.26 loaded **
GitHub repo: https://github.com/nanoframework/Home
Report issues: https://github.com/nanoframework/Home/issues
Browse samples: https://github.com/nanoframework/samples
Join our Discord community: https://discord.gg/gCyBu8T
Join our Hackster.io platform: https://www.hackster.io/nanoframework
Follow us on Twitter: https://twitter.com/nanoframework
Follow our YouTube channel: https://www.youtube.com/c/nanoFramework
Star our GitHub repos: https://github.com/nanoframework/Home
Add a short review or rate the VS extension: https://marketplace.visualstudio.com/items?itemName=nanoframework.nanoFramework-VS2022-Extension

ORGPAL_PALTHREE @ COM10 deployment area erased.
ORGPAL_PALTHREE @ COM10 deployment area erased.

System Information
HAL build info: nanoCLR running @ ORGPAL_PALTHREE built with ChibiOS v2021.11.1.11
  Target:   ORGPAL_PALTHREE
  Platform: STM32F7

Firmware build Info:
  Date:        Mar  9 2022
  Type:        MinSizeRel build with ChibiOS v2021.11.1.11
  CLR Version: 1.7.4.92
  Compiler:    GNU ARM GCC v10.3.1

OEM Product codes (vendor, model, SKU): 0, 0, 0

Serial Numbers (module, system):
  00000000000000000000000000000000
  0000000000000000

Target capabilities:
  Has nanoBooter: YES
  nanoBooter: v1.7.4.92
  IFU capable: NO
  Has proprietary bootloader: NO

AppDomains:

Assemblies:
  StringBuilderTests, 1.0.0.0
  nanoFramework.System.Text, 1.1.3.0
  mscorlib, 1.12.0.0

Native Assemblies:
  mscorlib v100.5.0.17, checksum 0x004CF1CE
  nanoFramework.Runtime.Native v100.0.9.0, checksum 0x109F6F22
  nanoFramework.Hardware.Stm32 v100.0.4.4, checksum 0x0874B6FE
  nanoFramework.Networking.Sntp v100.0.4.4, checksum 0xE2D9BDED
  nanoFramework.ResourceManager v100.0.0.1, checksum 0xDCD7DF4D
  nanoFramework.System.Collections v100.0.1.0, checksum 0x2DC2B090
  nanoFramework.System.Text v100.0.0.1, checksum 0x8E6EB73D
  nanoFramework.Runtime.Events v100.0.8.0, checksum 0x0EAB00C9
  EventSink v1.0.0.0, checksum 0xF32F4C3E
  System.IO.FileSystem v1.0.0.0, checksum 0x3AB74021
  System.Math v100.0.5.4, checksum 0x46092CB1
  System.Net v100.1.4.1, checksum 0xA01012C3
  Windows.Devices.Adc v100.1.3.3, checksum 0xCA03579A
  System.Device.Adc v100.0.0.0, checksum 0xE5B80F0B
  System.Device.Dac v100.0.0.6, checksum 0x02B3E860
  System.Device.Gpio v100.1.0.4, checksum 0xB6D0ACC1
  Windows.Devices.Gpio v100.1.2.2, checksum 0xC41539BE
  Windows.Devices.I2c v100.2.0.2, checksum 0x79EDBF71
  System.Device.I2c v100.0.0.1, checksum 0xFA806D33
  Windows.Devices.Pwm v100.1.3.3, checksum 0xBA2E2251
  System.Device.Pwm v100.1.0.4, checksum 0xABF532C3
  Windows.Devices.SerialCommunication v100.1.1.2, checksum 0x34BAF06E
  System.IO.Ports v100.1.4.0, checksum 0xCB7C0ECA
  Windows.Devices.Spi v100.1.4.2, checksum 0x360239F1
  System.Device.Spi v100.1.1.0, checksum 0x3F6E2A7E
  Windows.Storage v100.0.2.0, checksum 0x954A4192

++++++++++++++++++++++++++++++++
++        Memory Map          ++
++++++++++++++++++++++++++++++++
  Type     Start       Size
++++++++++++++++++++++++++++++++
  RAM   0xd0000000  0x00800000
  FLASH 0x08000000  0x00200000

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++                   Flash Sector Map                        ++
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  Region     Start      Blocks   Bytes/Block    Usage
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
      0    0x08000000       1      0x008000     nanoBooter
      1    0x08008000       1      0x008000     Configuration
      2    0x08010000       2      0x008000     nanoCLR
      3    0x08020000       1      0x020000     nanoCLR
      4    0x08040000       2      0x040000     nanoCLR
      5    0x080C0000       5      0x040000     Deployment

+++++++++++++++++++++++++++++++++++++++++++++++++++
++              Storage Usage Map                ++
+++++++++++++++++++++++++++++++++++++++++++++++++++
  Start        Size (kB)           Usage
+++++++++++++++++++++++++++++++++++++++++++++++++++
  0x08000000    0x008000 (32kB)     nanoBooter
  0x08008000    0x008000 (32kB)     Configuration
  0x08010000    0x0B0000 (704kB)    nanoCLR
  0x080C0000    0x140000 (1280kB)   Deployment

Deployment Map
Empty

Description

When using a fixed capacity, StringBuilder does not output. e.g. StringBuilder(256);

How to reproduce

Run the sample program.

Expected behaviour

The testStr SB Fixed Size: should also contain 0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ

Screenshots

image

Sample project or code

using System.Diagnostics;
using System.Threading;
using System.Text;

namespace StringBuilderTests
{
    internal static class Program
    {
        internal static void Main()
        {
            string testStr = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";

            Debug.WriteLine("Starting String Tests");
            Debug.WriteLine("");

            Debug.WriteLine("Just adding test string to debug, Writeline...");
            Debug.WriteLine($"testStrAdd: {testStr}");
            Debug.WriteLine("Complete! Should have shown: 0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ");
            Debug.WriteLine("");

            Debug.WriteLine("Start SB Variable");
            var sbvariable = new StringBuilder();
            sbvariable.Append("testStr SB Variable Size: ");
            sbvariable.Append(testStr);
            Debug.WriteLine(sbvariable.ToString());
            Debug.WriteLine("Complete! Should have shown: 0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ");
            Debug.WriteLine("");

            Debug.WriteLine("Start SB Fixed");
            var sbfixed = new StringBuilder(256);
            sbfixed.Append("testStr SB Fixed Size: ");
            sbvariable.Append(testStr);
            Debug.WriteLine(sbfixed.ToString());
            Debug.WriteLine("Complete! Should have shown: 0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ");
            Debug.WriteLine("");

            Thread.Sleep(Timeout.Infinite);
        }
    }
}

Aditional information

Looking at the StringBuilder Tests, they only cover the empty constructer.

AdrianSoundy commented 2 years ago

@networkfusion Your test code is causing the problem. you have:

sbvariable.Append(testStr);

when it should be:

sbfixed.Append(testStr);

Also i think the constructor StringBuilder(256) is for initial size not fixed size.

networkfusion commented 2 years ago

😱 , not sure how I missed that... Thanks. I will retest...

josesimoes commented 2 years ago

Any update on this?

networkfusion commented 2 years ago

Happy to close, it works now (if not a copy and paste error originally!). image

networkfusion commented 2 years ago

Actually, changed my mind on closing... lets make sure the unit tests cover fixed size first!

        internal static void Main()
        {
            string testStr = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";

            Debug.WriteLine("Starting String Tests");
            Debug.WriteLine("");

            Debug.WriteLine("Just adding test string to debug, Writeline...");
            Debug.WriteLine($"testStrAdd: {testStr}");
            Debug.WriteLine("Complete! Should have shown: 0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ");
            Debug.WriteLine("");

            Debug.WriteLine("Start SB Variable");
            var sbvariable = new StringBuilder();
            sbvariable.Append("testStr SB Variable Size: ");
            sbvariable.Append(testStr);
            Debug.WriteLine(sbvariable.ToString());
            Debug.WriteLine("Complete! Should have shown: 0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ");
            Debug.WriteLine("");

            Debug.WriteLine("Start SB Fixed");
            var sbfixed = new StringBuilder(256);
            sbfixed.Append("testStr SB Fixed Size: ");
            sbfixed.Append(testStr);
            Debug.WriteLine(sbfixed.ToString());
            Debug.WriteLine("Complete! Should have shown: 0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ");
            Debug.WriteLine("");

            Thread.Sleep(Timeout.Infinite);
        }
oblaise commented 2 years ago

Is it really fixed size capacity or is it like in .Net the allocation size of the first chunk but StringBuilder will add additional memory if required ? I know that the official documentation of Microsoft is confusing to this regard. You can check the test on .NET 6 at https://dotnetfiddle.net/M9Tg1X

AdrianSoundy commented 2 years ago

@oblaise I think it is both. The intial chunk size and max allocation. Really to check if you get an exception when you try to append more than max.

josesimoes commented 2 years ago

It is that yes, initial size and it will add as needed. For reference: https://github.com/nanoframework/System.Text/blob/main/nanoFramework.System.Text/Text/StringBuilder.cs

Questions: is there anything wrong with our implementation of it? Do we have any incompatibility with the full .NET implementation that we should fix or make it clear about it?

oblaise commented 2 years ago

@josesimoes No my misunderstanding came from the description in the bug above.