Fix inverted calculation of RTL text percentage in bidi #7948
Conversation
Skaty
changed the title
|
Looks good, but do you have a unit test you can add so we don't regress this? |
|
Do you want me to add the unit test just for this first? I have been doing up the basic unit tests for bidi, but I feel that this needs to be address as it's a breaking bug. |
Having tests will help to avoid breaking your fix in the future. Unit tests will be great to have. |
|
btw, bidi.js is one of the two core files that don't have unit tests (see #7261). |
|
I agree with that. I'm sorry if the question was not clear. I was resolving #7261 for bidi.js, but I found this particular issue that seems to break. So, should I add the full suite of unit tests that was meant to resolve #7261, or as an interim, just tests to ensure that this particular logic (percentage of RTL text) doesn't regress? |
The single test is fine as a starting point. Besides regressions, it helps prove the validity of the patch. |
Skaty
force-pushed
the
fix-bidi-fraction
branch
from
25784be to
c65aa66
Compare
|
Unit test for this patch has been added. |
|
/botio unittest |
From: Bot.io (Linux)ReceivedCommand cmd_unittest from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/33c90dd6fab2884/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @yurydelendik received. Current queue size: 0 Live output at: http://107.22.172.223:8877/fbf95518ce69b65/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/33c90dd6fab2884/output.txt Total script time: 2.04 mins
|
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/fbf95518ce69b65/output.txt Total script time: 2.88 mins
|
|
@yurydelendik Do you mean test/unit/clitests.json? |
Yes. |
Skaty
force-pushed
the
fix-bidi-fraction
branch
from
c65aa66 to
857a5da
Compare
|
@yurydelendik Done. |
|
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/6aefd21f0930c61/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.22.172.223:8877/1ae1fc1582cbe94/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/1ae1fc1582cbe94/output.txt Total script time: 25.92 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/6aefd21f0930c61/output.txt Total script time: 26.93 mins
|
|
Thank you for the patch |
Fix inverted calculation of RTL text percentage in bidi
While I was doing up unit tests for the Unicode bi-directional function, I realised that the fraction to calculate the percentage of RTL text in the document is swapped (i.e. it was calculating the inverse of the percentage).
Due to this bug, all bidi strings passed will be tagged as predominantly RTL, despite meeting the requirements listed.