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

Fix C# unit tests #818

Closed
wants to merge 2 commits into from
Closed

Fix C# unit tests #818

wants to merge 2 commits into from

Conversation

rigdern
Copy link

@rigdern rigdern commented Oct 1, 2018

This consists of several fixes:

  • Some of the C++ functions called by the tests were missing WIN_EXPORT
  • It looks like Yoga was changed to round up after TestMeasureFuncWithFloat was written. Here's the function call that results in the rounding:

    yoga/yoga/Yoga.cpp

    Lines 4019 to 4026 in 357ca78

    YGRoundValueToPixelGrid(
    absoluteNodeRight,
    pointScaleFactor,
    (textRounding && hasFractionalWidth),
    (textRounding && !hasFractionalWidth)) -
    YGRoundValueToPixelGrid(
    absoluteNodeLeft, pointScaleFactor, false, textRounding),
    YGDimensionWidth);
  • The format of the result of the Print method was changed after TestPrint was written.

Fix C# unit tests
4f7e1fa 
This consists of several fixes:
  - Some of the C++ functions called by the tests were missing `WIN_EXPORT`
  - It looks like Yoga was changed to round up after `TestMeasureFuncWithFloat` was written. Here's the function call that results in the rounding: https://github.com/facebook/yoga/blob/357ca78f9f7bd86298ef00a1426cac5ca2041d3b/yoga/Yoga.cpp#L4019-L4026
  - The format of the result of the `Print` method was changed after `TestPrint` was written.
@rigdern
Copy link
Author

rigdern commented Oct 1, 2018

Questions

  • Should we use WIN_EXPORT on every public C function? My PR adds it to some that broke the tests but maybe we should just add it to all of them.
  • Are we supposed to use buck to run tests on Windows? It didn't work for me because it seemed to pass clang flags to Microsoft's C++ compiler.
  • There are some C#-specific tests. Which solution should I be opening for that?
    • Facebook.Yoga.Desktop.sln
    • Facebook.Yoga.sln
    • Facebook.Yoga.Universal.sln

/cc @mattpodwysocki @rozele @matthargett

shergin
shergin reviewed Oct 9, 2018
View reviewed changes
csharp/Windows/Facebook.Yoga.Universal.Tests/YogaNodeTest.cs Outdated
@@ -285,7 +290,7 @@ public void TestCopyStyle()
}

#if !UNITY_EDITOR
private void ForceGC()
private void ForceGC()
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we revert this line?

facebook-github-bot
facebook-github-bot reviewed Oct 10, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Oct 10, 2018
@facebook-github-bot
Fix C# unit tests (#818)
b747286 
Summary:
This consists of several fixes:
  - Some of the C++ functions called by the tests were missing `WIN_EXPORT`
  - It looks like Yoga was changed to round up after `TestMeasureFuncWithFloat` was written. Here's the function call that results in the rounding: https://github.com/facebook/yoga/blob/357ca78f9f7bd86298ef00a1426cac5ca2041d3b/yoga/Yoga.cpp#L4019-L4026
  - The format of the result of the `Print` method was changed after `TestPrint` was written.
Pull Request resolved: #818

Differential Revision: D10282902

Pulled By: shergin

fbshipit-source-id: a5fd732a7dbff1a704cbafbfc95ae3e0c0a0cdd8
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Oct 10, 2018
@facebook-github-bot
Fix C# unit tests (#818)
780ced4 
Summary:
This consists of several fixes:
  - Some of the C++ functions called by the tests were missing `WIN_EXPORT`
  - It looks like Yoga was changed to round up after `TestMeasureFuncWithFloat` was written. Here's the function call that results in the rounding: https://github.com/facebook/yoga/blob/357ca78f9f7bd86298ef00a1426cac5ca2041d3b/yoga/Yoga.cpp#L4019-L4026
  - The format of the result of the `Print` method was changed after `TestPrint` was written.
Pull Request resolved: facebook/yoga#818

Differential Revision: D10282902

Pulled By: shergin

fbshipit-source-id: a5fd732a7dbff1a704cbafbfc95ae3e0c0a0cdd8
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
@facebook-github-bot
Fix C# unit tests (#818)
f125291 
Summary:
This consists of several fixes:
  - Some of the C++ functions called by the tests were missing `WIN_EXPORT`
  - It looks like Yoga was changed to round up after `TestMeasureFuncWithFloat` was written. Here's the function call that results in the rounding: https://github.com/facebook/yoga/blob/357ca78f9f7bd86298ef00a1426cac5ca2041d3b/yoga/Yoga.cpp#L4019-L4026
  - The format of the result of the `Print` method was changed after `TestPrint` was written.
Pull Request resolved: facebook/yoga#818

Differential Revision: D10282902

Pulled By: shergin

fbshipit-source-id: a5fd732a7dbff1a704cbafbfc95ae3e0c0a0cdd8
CodeWitchBella pushed a commit to CodeWitchBella/yoga-wasm that referenced this pull request Sep 2, 2019
@facebook-github-bot
Fix C# unit tests (#818)
b28c8f0 
Summary:
This consists of several fixes:
  - Some of the C++ functions called by the tests were missing `WIN_EXPORT`
  - It looks like Yoga was changed to round up after `TestMeasureFuncWithFloat` was written. Here's the function call that results in the rounding: https://github.com/facebook/yoga/blob/357ca78f9f7bd86298ef00a1426cac5ca2041d3b/yoga/Yoga.cpp#L4019-L4026
  - The format of the result of the `Print` method was changed after `TestPrint` was written.
Pull Request resolved: facebook/yoga#818

Differential Revision: D10282902

Pulled By: shergin

fbshipit-source-id: a5fd732a7dbff1a704cbafbfc95ae3e0c0a0cdd8
KusStar pushed a commit to KusStar/react-native that referenced this pull request Oct 28, 2020
@KusStar
Fix C# unit tests (facebook#818)
8d2e826 
Summary:
This consists of several fixes:
  - Some of the C++ functions called by the tests were missing `WIN_EXPORT`
  - It looks like Yoga was changed to round up after `TestMeasureFuncWithFloat` was written. Here's the function call that results in the rounding: https://github.com/facebook/yoga/blob/357ca78f9f7bd86298ef00a1426cac5ca2041d3b/yoga/Yoga.cpp#L4019-L4026
  - The format of the result of the `Print` method was changed after `TestPrint` was written.
Pull Request resolved: facebook/yoga#818

Differential Revision: D10282902

Pulled By: shergin

fbshipit-source-id: a5fd732a7dbff1a704cbafbfc95ae3e0c0a0cdd8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shergin shergin shergin left review comments

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rigdern @shergin @facebook-github-bot