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

Ensure tests run linux and macOS #447

Merged
merged 4 commits into from
Dec 8, 2020

Conversation

macite
Copy link
Contributor

@macite macite commented Oct 25, 2020

This PR fixes a few small issues with unit tests that result in tests not being able to run on Linux and macOS using dotnet core.

  • Fixes an infinite loop resulting from replacing null terminator within strings
  • Registers the code page provider within the CodePageUtil to ensure availability in dotnet standard
  • Corrects missing checks in the test sanity checker and removes unsafe thread usage

It would be much appreciated if you could add the hacktoberfest-accepted label to this PR. I believe that this is a valid contribution to the NPOI project, and this label will indicate this to the Hacktoberfest event.

On unix this search returns true for any index and any string value - as it searches for 0 characters before the null terminator. Replacing this string with a char corrects the issue and will work in the same way across mac, windows, and linux.
This was causing issues when testing on unix.
Check was not being assigned a value, and resulted
in a null reference exception when run. This change
fixes the missing check data, and removes the use
of Threads, as data is not passed in a safe manner
between the calling thread and the created thread.
Without this tests fail due to lack of encoding provider.
@tonyqus tonyqus added this to the NPOI 2.5.2 milestone Oct 27, 2020
@tonyqus tonyqus modified the milestones: NPOI 2.5.2, NPOI 2.5.3 Nov 25, 2020
@tonyqus tonyqus modified the milestones: NPOI 2.5.5, NPOI 2.5.3 Dec 8, 2020
@tonyqus tonyqus merged commit 0f63338 into nissl-lab:master Dec 8, 2020
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.

2 participants