search

barnhill / barcodelib

C# Barcode Image Generation Library
Apache License 2.0
736 stars 239 forks source link

Add tests for CODE128 and support netcoreapp5.0. #124

Closed binki closed 3 years ago

binki commented 3 years ago

Preparation for changes for #123.

barnhill commented 3 years ago

Not for this or but should probably enable unit test runs on PRs in GitHub actions

binki commented 3 years ago

Not for this or but should probably enable unit test runs on PRs in GitHub actions

I did add dotnet test to the CI thing in this PR. You can see “Tests” under https://github.com/barnhill/barcodelib/runs/1898303929 . I don’t know very much about GitHub actions—maybe GitHub has some way to actually be aware of the test results. I think that just running dotnet test is enough to make the CI fail if the test fail, though…?

What would be interesting/useful is figuring out how to get the tests to target both netcoreapp3.1 and netcoreapp5.0. Hmm.

binki commented 3 years ago

An interesting thing you can see in https://github.com/barnhill/barcodelib/runs/1899841830 is that the netcoreapp3.1 fails on the GitHub Actions system because the Linux version of netcoreapp3.1 uses the “new” platform independent code already. If you run netcoreapp3.1 on Windows, it succeeds because that version uses the Windows stuff (as described in #123) :

C:\Users\ohnob\repos\barcodelib>dotnet test
  Determining projects to restore...
  Restored C:\Users\ohnob\repos\barcodelib\BarcodeStandardExample\BarcodeStandardExample.csproj (in 100 ms).
  2 of 3 projects are up-to-date for restore.
  BarcodeStandard -> C:\Users\ohnob\repos\barcodelib\BarcodeStandard\bin\Debug\netstandard2.0\BarcodeStandard.dll
  Successfully created package 'C:\Users\ohnob\repos\barcodelib\BarcodeStandard\bin\Debug\BarcodeLib.2.2.10.nupkg'.
  Successfully created package 'C:\Users\ohnob\repos\barcodelib\BarcodeStandard\bin\Debug\BarcodeLib.2.2.10.snupkg'.
  BarcodeStandardTests -> C:\Users\ohnob\repos\barcodelib\BarcodeStandardTests\bin\Debug\net472\BarcodeStandardTests.dll
  BarcodeStandardTests -> C:\Users\ohnob\repos\barcodelib\BarcodeStandardTests\bin\Debug\netcoreapp3.1\BarcodeStandardTests.dll
  BarcodeStandardTests -> C:\Users\ohnob\repos\barcodelib\BarcodeStandardTests\bin\Debug\netcoreapp5.0\BarcodeStandardTests.dll
Test run for C:\Users\ohnob\repos\barcodelib\BarcodeStandardTests\bin\Debug\netcoreapp3.1\BarcodeStandardTests.dll (.NETCoreApp,Version=v3.1)
Microsoft (R) Test Execution Command Line Tool Version 16.8.3
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

Passed!  - Failed:     0, Passed:    11, Skipped:     0, Total:    11, Duration: 98 ms - BarcodeStandardTests.dll (netcoreapp3.1)
Test run for C:\Users\ohnob\repos\barcodelib\BarcodeStandardTests\bin\Debug\netcoreapp5.0\BarcodeStandardTests.dll (.NETCoreApp,Version=v5.0)
Microsoft (R) Test Execution Command Line Tool Version 16.8.3
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
  Failed EncodeBarcode [111 ms]
  Failed EncodeBarcode (↕,1101000010010010011110100100111101100011101011,1101000010010010011110100100111101100011101011,,) [39 ms]
  Error Message:
   Assert.AreEqual failed. Expected:<1101000010010010011110100100111101100011101011>. Actual:<1101000010011011001100110110011001100011101011>. CODE128
  Stack Trace:
     at BarcodeStandardTests.Symbologies.Code128Tests.<EncodeBarcode>g__assertByType|1_0(TYPE type, String expected, <>c__DisplayClass1_0& ) in C:\Users\ohnob\repos\barcodelib\BarcodeStandardTests\Symbologies\Code128Tests.cs:line 48
   at BarcodeStandardTests.Symbologies.Code128Tests.EncodeBarcode(String data, String expectedAuto, String expectedA, String expectedB, String expectedC) in C:\Users\ohnob\repos\barcodelib\BarcodeStandardTests\Symbologies\Code128Tests.cs:line 50

  Failed EncodeBarcode (¶,1101000010010011110100100111101001100011101011,1101000010010011110100100111101001100011101011,,) [< 1 ms]
  Error Message:
   Assert.AreEqual failed. Expected:<1101000010010011110100100111101001100011101011>. Actual:<1101000010011011001100110110011001100011101011>. CODE128
  Stack Trace:
     at BarcodeStandardTests.Symbologies.Code128Tests.<EncodeBarcode>g__assertByType|1_0(TYPE type, String expected, <>c__DisplayClass1_0& ) in C:\Users\ohnob\repos\barcodelib\BarcodeStandardTests\Symbologies\Code128Tests.cs:line 48
   at BarcodeStandardTests.Symbologies.Code128Tests.EncodeBarcode(String data, String expectedAuto, String expectedA, String expectedB, String expectedC) in C:\Users\ohnob\repos\barcodelib\BarcodeStandardTests\Symbologies\Code128Tests.cs:line 50

  Failed EncodeBarcode (this↕is¶weird,110100100001001111010010011000010100001101001011110010011101011110100100111101011110111010000110100101111001001110101111010011110100101111011101111001010010110010000100001101001001001111010000100110101100010001100011101011,,,) [1 ms]
  Error Message:
   Assert.AreEqual failed. Expected:<110100100001001111010010011000010100001101001011110010011101011110100100111101011110111010000110100101111001001110101111010011110100101111011101111001010010110010000100001101001001001111010000100110101100010001100011101011>. Actual:<110100100001001111010010011000010100001101001011110010011101011110110110011001011110111010000110100101111001001110101111011011001100101111011101111001010010110010000100001101001001001111010000100110111100010101100011101011>. CODE128
  Stack Trace:
     at BarcodeStandardTests.Symbologies.Code128Tests.<EncodeBarcode>g__assertByType|1_0(TYPE type, String expected, <>c__DisplayClass1_0& ) in C:\Users\ohnob\repos\barcodelib\BarcodeStandardTests\Symbologies\Code128Tests.cs:line 48
   at BarcodeStandardTests.Symbologies.Code128Tests.EncodeBarcode(String data, String expectedAuto, String expectedA, String expectedB, String expectedC) in C:\Users\ohnob\repos\barcodelib\BarcodeStandardTests\Symbologies\Code128Tests.cs:line 50
binki commented 3 years ago

@barnhill I am somewhat confident in this and it might be nice to get a release out which supports netcoreapp5.0 since, with its rebranding as net5.0, that might become popular.

@jangernert sorry for stealing this from you. it just seemed somewhat interesting x.x. When you get time, please criticize/test these changes!

barnhill commented 3 years ago

Awesome 👍. Let me have a look and build later then get a release out if it's good. I'm off work tomorrow so it's the right timing

jangernert commented 3 years ago

@binki No problem.

The only thing I don't quite understand is the need for the Code128Entry struct. Wouldn't it be enough to have a List<string> with the encodings?

binki commented 3 years ago

@jangernert

@binki No problem.

The only thing I don't quite understand is the need for the Code128Entry struct. Wouldn't it be enough to have a List<string> with the encodings?

I was just doing the minimal code changes to get rid of DataTable—so I just recreated the table with types instead of trying to figure out how things worked.

It looks like being able to look up the symbolic names (i.e., the A, B, and C columns) was used in InsertStartandCodeCharacters() when inserting the START_A, START_B, or START_C initial bar sequence and when inserting a code change such as CODE_A, CODE_B, or CODE_C. I updated this to use dictionary lookups instead and removed the struct. Further work could definitely be done if making this more efficient is desired. These very dictionary lookups could be avoided if it is refactored to just pass around the right data in the right place instead of having one part of the code reverse-solve the results another gives. That would probably be good to do and I’m sure PRs would be accepted. For me, it just isn’t interesting enough and I need to spend time on other things x.x.

jangernert commented 3 years ago

At least I'm happy with the code now. This is already a big improvement over the current situation imo. There is almost always room for further improvements. That shouldn't hold back a PR until it solves everything perfectly.

But that's just my opinion.

barnhill commented 3 years ago

I agree. I was reviewing this when the internet here went out :(

Brad Barnhill, MSN, RN bradbarnhill@hotmail.com

On Mon, Feb 15, 2021, 9:42 AM Jan Lukas Gernert notifications@github.com wrote:

At least I'm happy with the code now. This is already a big improvement over the current situation imo. There is almost always room for further improvements. That shouldn't hold back a PR until it solves everything perfectly.

But that's just my opinion.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/barnhill/barcodelib/pull/124#issuecomment-779302627, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5S2DVBWPOODS75MHH6M73S7E6GVANCNFSM4XTMEDUQ .

jangernert commented 3 years ago

Any progress on this?

barnhill commented 3 years ago

Out of town till Saturday ... I won't be able to build and release to nugget till I get back. Got the ol work Mac with me.

binki commented 3 years ago

@barnhill did you get a chance to review this yet?

barnhill commented 3 years ago

Oh man I'm sorry!! I'll review it.

barnhill commented 3 years ago

Ran tests and manually tested encoding C128 barcodes and reading them.