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.

Sign up for GitHub

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

Jump to bottom

Test unicode symbols #1186

Merged
merged 31 commits into from
Mar 23, 2023
Merged

Test unicode symbols #1186

merged 31 commits into from
Mar 23, 2023

Conversation

desistefanova
Copy link
Contributor

@desistefanova desistefanova commented Feb 28, 2023 •
edited
Loading

Add new tests with unicode symbols.
Generated new strings for test emails using the allowed symbols for email.
Fixes #1191
Depends on core release:
realm/realm-core#6350
realm/realm-core#6336

@cla-bot cla-bot bot added the cla: yes label Feb 28, 2023
@desistefanova desistefanova changed the base branch from main to upgrade_core_13.5.0 February 28, 2023 10:50
@desistefanova desistefanova added the no-changelog Used to skip the changelog check label Feb 28, 2023
@sync-by-unito sync-by-unito bot mentioned this pull request Mar 1, 2023
Closed
Base automatically changed from upgrade_core_13.5.0 to main March 6, 2023 08:34
@desistefanova desistefanova changed the base branch from main to ds/upgrade_core_13.6.0 March 6, 2023 23:42
@desistefanova desistefanova marked this pull request as ready for review March 7, 2023 12:53
Base automatically changed from ds/upgrade_core_13.6.0 to main March 13, 2023 12:59
@desistefanova desistefanova marked this pull request as draft March 13, 2023 13:15
@desistefanova desistefanova marked this pull request as ready for review March 13, 2023 13:27
blagoev
blagoev reviewed Mar 16, 2023
View reviewed changes
Copy link
Contributor

@blagoev blagoev left a comment

Choose a reason for hiding this comment

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

the tests seem fine but the generate method changes needs refactoring

@desistefanova
Revert "Merge branch 'main' into ds/unicode_file_path"
3886603 
This reverts commit b9001e8, reversing
changes made to 381a624.

Revert "Merge branch 'main' into ds/unicode_file_path"

This reverts commit b9001e8, reversing
changes made to 999871e.
@desistefanova
Code review changes
0dd7de7
@desistefanova
Remove the usage of an old method
72ce82f
@desistefanova desistefanova requested a review from blagoev March 22, 2023 12:09
nielsenko
nielsenko reviewed Mar 22, 2023
View reviewed changes
Copy link
Contributor

@nielsenko nielsenko left a comment

Choose a reason for hiding this comment

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

One thing. There is not guarantee that the generateRandomString function actually return a string with non-ascii characters, when the unicode parameter is set to true. It is likely, but not guaranteed. Also, ascii characters are part of unicode, so perhaps the flag could have a better name.

@desistefanova
Copy link
Contributor Author

One thing. There is not guarantee that the generateRandomString function actually return a string with non-ascii characters, when the unicode parameter is set to true. It is likely, but not guaranteed. Also, ascii characters are part of unicode, so perhaps the flag could have a better name.

Ok, I changed the name to allowUnicode, which means not only ASCI characters could be expected.

@desistefanova desistefanova requested a review from nielsenko March 22, 2023 12:38
blagoev
blagoev suggested changes Mar 23, 2023
View reviewed changes
@desistefanova desistefanova requested a review from blagoev March 23, 2023 13:52
@desistefanova desistefanova merged commit b983b30 into main Mar 23, 2023
@desistefanova desistefanova deleted the ds/unicode_file_path branch March 23, 2023 16:13
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@blagoev blagoev blagoev approved these changes

@nielsenko nielsenko Awaiting requested review from nielsenko

Assignees

@desistefanova desistefanova

Labels
cla: yes no-changelog Used to skip the changelog check
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test unicode symbols
3 participants
@desistefanova @blagoev @nielsenko