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

Include test case for special characters #103

Merged
merged 6 commits into from
Jan 2, 2025

Conversation

aarsilv
Copy link
Contributor

@aarsilv aarsilv commented Dec 30, 2024

Eppo Internal:
🎟️ Ticket: FF-3763 - Update shared SDK test data to have a test case for special characters
🗨️ Slack Thread: "...flag text encoding problem..."

Description:
SDKs need to be able to handle special, non-ASCII characters. This PR adds a test that checks that by using strings with characters from a variety of languages as well as emojis, which all have non-ASCII values.

Copy link
Collaborator

@rasendubi rasendubi left a comment

Choose a reason for hiding this comment

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

Thanks for adding the test cases

There's one caveat that this likely won't help right now as languages having issues with encoding likely will carry it over to their test cases as well. e2e testing effort should be helpful here

Comment on lines 1199 to 1202
"ru": {
"key": "ru",
"value": "{\"a\": \"заботится\", \"b\": \"красивый\"}"
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙂

Suggested change
"ru": {
"key": "ru",
"value": "{\"a\": \"заботится\", \"b\": \"красивый\"}"
},
"ua": {
"key": "ua",
"value": "{\"a\": \"піклуватися\", \"b\": \"любов\"}"
},

@aarsilv
Copy link
Contributor Author

aarsilv commented Dec 31, 2024

There's one caveat that this likely won't help right now as languages having issues with encoding likely will carry it over to their test cases as well.

Can you elaborate on this? The encoded obfuscated file was done correctly (our UFC generation in node uses Buffer), so any languages that decode incorrectly will fail. For example, the JavaScript client test failed with these fixtures until I implemented the fix.

@rasendubi
Copy link
Collaborator

Can you elaborate on this? The encoded obfuscated file was done correctly (our UFC generation in node uses Buffer), so any languages that decode incorrectly will fail. For example, the JavaScript client test failed with these fixtures until I implemented the fix.

Sorry, I haven't checked the fix before commenting. What I meant is that if an SDK doesn't use UTF-8 to read server response, it will likely to read the test file incorrectly as well. That's not the issue though, so please ignore my comment 🙂

@aarsilv aarsilv merged commit 2eea02c into main Jan 2, 2025
31 of 39 checks passed
@aarsilv aarsilv deleted the aaron/ff-3763/special-characters-test branch January 2, 2025 17:51
maya-the-mango pushed a commit that referenced this pull request Jan 17, 2025
* test case for special characters

* update obfuscated config

* update names in description

* include unevaluated allocation

* update cyrillic language example

* fix reason
maya-the-mango added a commit that referenced this pull request Jan 31, 2025
* nest app boilerplate for node sdk relay

* nest app boilerplate for node sdk relay

* nest app boilerplate for node sdk relay improvements

* assignment handling WIP

* get assignment handling proxy

* get assignment handling proxy

* get assignment handling proxy

* get bandits handling proxy

* Include test case for special characters (#103)

* test case for special characters

* update obfuscated config

* update names in description

* include unevaluated allocation

* update cyrillic language example

* fix reason

* build and run + workflows for node-server relay

* review changes

* review changes

* feat: React Native Application using Eppo SDK (#106)

* inital app from create-expo-app
* add Eppo SDK; traditional and precomputed client
* Eppo Client provider and basic flag/bandit display

* fix: workflow typo (#109)

* fixed type injection for node-server-sdk relay app

* updated README.md

* build and run update

* nest app boilerplate for node sdk relay

* nest app boilerplate for node sdk relay

* nest app boilerplate for node sdk relay improvements

* assignment handling WIP

* get assignment handling proxy

* get assignment handling proxy

* get assignment handling proxy

* get bandits handling proxy

* build and run + workflows for node-server relay

* review changes

* review changes

* fixed type injection for node-server-sdk relay app

* updated README.md

* build and run update

---------

Co-authored-by: Aaron Silverman <[email protected]>
Co-authored-by: Tyler Potter <[email protected]>
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.

4 participants