Support step definitions with multi-byte characters #224

rkk-ableton · 2019-06-17T09:42:30Z

Summary

This branch adds support for handling characters outside of the Unicode block 'Basic Latin' by updating json-spirit to the latest version and modifying cucumber-cpp to properly handle this change.

Motivation and Context

This change adds support for passing raw UTF-8 strings from cucumber-cpp to cucumber-ruby which is necessary when writing tests which use non-'Basic Latin' characters to validate application behavior.

This relates to and resolves #40.

How Has This Been Tested?

This branch adds three new tests (5fff48f).

These tests were run following cucumber-cpp's own instructions for building and testing.

While commit 5fff48f is checked out, the tests, which demonstrate incorrect behavior, pass.
Commit 581b3c8 updates the WireProtocolTest in addition to modifying the WireResponseEncoder::encode. The modified test, demonstrating correct behavior, passes.
Commit 1c0a1bd modifies the other two tests to demonstrate the corrected RegEx match results.

Types of changes

Bug fix (non-breaking change which fixes an issue).
New feature (non-breaking change which adds functionality).
Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

It is my own work, its copyright is implicitly assigned to the project and no substantial part of it has been copied from other sources (including Stack Overflow). In rare occasions this is acceptable, like in CMake modules where the original copyright information should be kept.
I'm using the same code standards as the existing code (indentation, spacing, variable naming, ...).
I've added tests for my code.
I have verified whether my change requires changes to the documentation
My change either requires no documentation change or I've updated the documentation accordingly.
My branch has been rebased to master, keeping only relevant commits.

coveralls · 2019-06-17T10:09:19Z

Coverage increased (+0.03%) to 62.503% when pulling 1c0a1bd on AbletonAppDev:update-json-spirit-upstream into dd424c1 on cucumber:master.

json_spirit's escaping of multibyte characters creates bugs in the WireProtocol which prevent usage of valid UTF-8 encoded characters in step definitions relying on RegEx. The new tests: /features/specific/wire_encoding.feature /tests/integration/WireProtocolTest.cpp /tests/unit/RegexTest.cpp

rkk-ableton · 2019-06-28T15:52:56Z

RFC @muggenhor & @paoloambrosio.

paoloambrosio

Thanks for the great contribution! This should fix #40.

On top of the other comments, can you remove the executable flag? All json_spirit files went from 644 to 755.

travis.sh

src/Regex.cpp

This change updates json-spirit to the latest public version: https://www.codeproject.com/KB/recipes/JSON_Spirit/json_spirit_v4.08.zip 4.08 adds support for a raw_utf8 option when writing a JSON string. Previously, multibyte characters were being escaped when being sent from cucumber-cpp to cucumber-ruby. Because cucumber-ruby's wire decoder does not properly decode escaped character sequences, this would crash cucumber-ruby.

This modifies the WireResponseEncoder to always use the raw_utf8 option provided by the new version of json_spirit. According to the IETF RFC8259: "JSON text exchanged between systems that are not part of a closed ecosystem MUST be encoded using UTF-8 [RFC3629]." https://tools.ietf.org/html/rfc8259

rkk-ableton · 2019-07-01T10:58:57Z

All json_spirit files went from 644 to 755.

@paoloambrosio I fixed up the commit updating json_spirit. This should now be resolved.

`cucumber-ruby` expects position values which are based on the index of the codepoint instead of the index of the code unit. This change modifies the value returned to `cucumber-ruby`. Prior to this change, the RegexSubMatch's position, which was correct in terms of a code unit array, would cause an `index out of string` error and crash cucumber-ruby when pretty-printing the results of a test. This commit also ammends the added tests to demonstrate the corrected behavior.

rkk-ableton · 2019-07-08T10:20:49Z

Hi @paoloambrosio,

I believe the change you requested has been addressed. Is there anything else you'd like changed before this is merged?

Thank you!

rkk-ableton · 2019-08-05T09:17:33Z

Hi @paoloambrosio, I wanted to try and ping you one more time to check if you'd like any more changes to the branch prior to merging.

Thanks!

rkk-ableton · 2021-07-22T14:52:26Z

Hi @muggenhor, @paoloambrosio, @konserw, this issue has come up again. Would you consider merging this PR?

paoloambrosio · 2021-07-26T07:20:59Z

I'm not maintaining this repo anymore. From a quick scan it looks good to me. I'll leave it to the new maintainer @jermus67 to review and merge.

aslakhellesoy · 2021-08-04T16:23:05Z

Hi @src-ableton,

Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾

In return for this generous offer we hope you will:

✅ Continue to use branches and pull requests. When someone on the core team approves a pull request (yours or someone else's), you're welcome to merge it yourself.
💚 Commit to setting a good example by following and upholding our code of conduct in your interactions with other collaborators and users.
💬 Join the community Slack channel to meet the rest of the team and make yourself at home.
ℹ️ Don't feel obliged to help, just do what you can if you have the time and the energy.
🙋 Ask if you need anything. We're looking for feedback about how to make the project more welcoming, so please tell us!

On behalf of the Cucumber core team,
Aslak Hellesøy
Creator of Cucumber

rkk-ableton · 2021-08-23T10:00:54Z

@aslakhellesoy Thanks for the invitiation to the organization! I was unable to accept, however, having been on vacation the last three weeks. Could you please re-send the invite?

aslakhellesoy · 2021-08-24T12:00:43Z

@src-ableton done!

rkk-ableton force-pushed the update-json-spirit-upstream branch from 1824e1a to b0ba33b Compare June 17, 2019 12:36

rkk-ableton changed the title ~~Update json-spirit~~ Update json-spirit to 4.08 Jun 17, 2019

rkk-ableton force-pushed the update-json-spirit-upstream branch 5 times, most recently from d846b42 to 73007d1 Compare June 18, 2019 10:14

rkk-ableton marked this pull request as ready for review June 18, 2019 10:20

rkk-ableton force-pushed the update-json-spirit-upstream branch from 73007d1 to b7c3a96 Compare June 19, 2019 11:51

rkk-ableton force-pushed the update-json-spirit-upstream branch from b7c3a96 to 135d9c3 Compare June 28, 2019 14:05

rkk-ableton force-pushed the update-json-spirit-upstream branch from 135d9c3 to c1f67b8 Compare June 28, 2019 14:08

rkk-ableton changed the title ~~Update json-spirit to 4.08~~ Support step definitions with multi-byte characters Jun 28, 2019

paoloambrosio reviewed Jun 29, 2019

View reviewed changes

travis.sh Show resolved Hide resolved

src/Regex.cpp Outdated Show resolved Hide resolved

rkk-ableton force-pushed the update-json-spirit-upstream branch from a35ec16 to ae6c54e Compare July 1, 2019 10:24

src added 2 commits July 1, 2019 12:52

rkk-ableton force-pushed the update-json-spirit-upstream branch from ae6c54e to 02d74ed Compare July 1, 2019 10:53

rkk-ableton force-pushed the update-json-spirit-upstream branch 2 times, most recently from 74c1c8a to 615581f Compare July 1, 2019 13:25

rkk-ableton force-pushed the update-json-spirit-upstream branch from 615581f to 1c0a1bd Compare July 2, 2019 08:01

paoloambrosio approved these changes Jul 26, 2021

View reviewed changes

jermus67 approved these changes Aug 4, 2021

View reviewed changes

jermus67 merged commit 445d4f8 into cucumber:main Aug 4, 2021

rkk-ableton mentioned this pull request Aug 24, 2021

Sync with upstream [24.08.2021] AbletonAppDev/cucumber-cpp#10

Merged

9 tasks

ala-ableton deleted the update-json-spirit-upstream branch August 24, 2021 11:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support step definitions with multi-byte characters #224

Support step definitions with multi-byte characters #224

rkk-ableton commented Jun 17, 2019 •

edited

Loading

coveralls commented Jun 17, 2019 •

edited

Loading

rkk-ableton commented Jun 28, 2019

paoloambrosio left a comment

rkk-ableton commented Jul 1, 2019

rkk-ableton commented Jul 8, 2019

rkk-ableton commented Aug 5, 2019

rkk-ableton commented Jul 22, 2021

paoloambrosio commented Jul 26, 2021

aslakhellesoy commented Aug 4, 2021

rkk-ableton commented Aug 23, 2021

aslakhellesoy commented Aug 24, 2021

Support step definitions with multi-byte characters #224

Support step definitions with multi-byte characters #224

Conversation

rkk-ableton commented Jun 17, 2019 • edited Loading

Summary

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

coveralls commented Jun 17, 2019 • edited Loading

rkk-ableton commented Jun 28, 2019

paoloambrosio left a comment

Choose a reason for hiding this comment

rkk-ableton commented Jul 1, 2019

rkk-ableton commented Jul 8, 2019

rkk-ableton commented Aug 5, 2019

rkk-ableton commented Jul 22, 2021

paoloambrosio commented Jul 26, 2021

aslakhellesoy commented Aug 4, 2021

rkk-ableton commented Aug 23, 2021

aslakhellesoy commented Aug 24, 2021

rkk-ableton commented Jun 17, 2019 •

edited

Loading

coveralls commented Jun 17, 2019 •

edited

Loading