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

Add tests for handling invalid UTF-8 insertion strings in WelSupportedTest #346

Merged
merged 6 commits into from
Jun 17, 2023

Conversation

Boombag0607
Copy link
Contributor

Description

This pull request addresses issue #273. Adds new tests to the WelSupportedTest fixture in order to handle invalid UTF-8 insertion strings. The tests focus on various scenarios to ensure correct behaviour and error handling.

Test Cases

  1. InvalidUtf8InsertionString: This test verifies that an error is raised when an invalid UTF-8 string is provided as an insertion string.

  2. InsertionIndexNotModifiedWhenInvalidUtf8StringProvided: This test checks that the insertion string at the specified index remains unchanged when an invalid UTF-8 string is provided.

  3. InvalidUtf8InsertionStringOutOfRange: This test ensures that an error is raised when attempting to set an invalid UTF-8 string at an out-of-range index.

  4. InsertionIndexNotModifiedWhenOutOfRangeIndexProvided: This test verifies that the insertion string at a valid index remains unchanged when an out-of-range index is provided for an invalid UTF-8 string.

  5. InsertionIndexNotModifiedWhenInRangeButNotAssigned: This test confirms that an unassigned insertion string at a valid index returns nullptr.

@codecov
Copy link

codecov bot commented Jun 14, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (f114b73) 91.55% compared to head (dc79894) 91.55%.

Additional details and impacted files
@@           Coverage Diff           @@
##           latest     #346   +/-   ##
=======================================
  Coverage   91.55%   91.55%           
=======================================
  Files          43       43           
  Lines        3660     3660           
  Branches      474      474           
=======================================
  Hits         3351     3351           
  Misses        216      216           
  Partials       93       93           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@goatshriek goatshriek added the testing pertains to the test suite label Jun 15, 2023
@goatshriek
Copy link
Owner

I'm converting this to a draft for now just to make it clear that this is still in work. Once you're all finished with it or you need help, let me know and I'll review it.

@goatshriek goatshriek marked this pull request as draft June 15, 2023 11:54
@Boombag0607
Copy link
Contributor Author

Sure thing. The problem I'm facing is that I'm currently using a Mac M1 and since the test is particularly Windows related, I can't really find a way to compile the files/ run the workflows locally. There's really no way for me to know the checks before pushing. Can you suggest a workaround?

@goatshriek
Copy link
Owner

goatshriek commented Jun 15, 2023

Without having access to a Windows machine to work on, this will be a little more difficult for you. One possible workaround is to enable Github Actions on your fork, remove all workflows but the Windows ones, and push to it to run the Windows CI workflows when you need to test something. You'd need to be sure to add the original workflow files before the request is marked as ready.

You could also try something like Wine, but the library hasn't been tested on this and I wouldn't be able to offer any support for getting it working. Using Actions on your own fork is probably the easiest thing to do, so that you won't need to wait for me to approve runs on the upstream repository.

@Boombag0607 Boombag0607 force-pushed the test-1 branch 2 times, most recently from 5f86299 to 9fadd63 Compare June 15, 2023 18:44
@Boombag0607 Boombag0607 marked this pull request as ready for review June 16, 2023 19:02
Copy link
Owner

@goatshriek goatshriek left a comment

Choose a reason for hiding this comment

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

Thanks very much for putting these tests together, they are mostly ready! There are a few updates for you to make listed below, after which I will merge this in.

.github/workflows/cygwin.yml Outdated Show resolved Hide resolved
.github/workflows/windows.yml Outdated Show resolved Hide resolved
test/function/config/wel_supported.cpp Outdated Show resolved Hide resolved
test/function/config/wel_supported.cpp Show resolved Hide resolved
test/function/config/wel_supported.cpp Outdated Show resolved Hide resolved
@goatshriek goatshriek merged commit 288d97d into goatshriek:latest Jun 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing pertains to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants