From c93734f579b4d4d2a4968c157559504efd48c4e8 Mon Sep 17 00:00:00 2001 From: Joe Vilches Date: Tue, 5 Dec 2023 13:30:03 -0800 Subject: [PATCH] Fix issue in gentest where border- would add a border to test (#1496) Summary: Pull Request resolved: https://github.com/facebook/yoga/pull/1496 Gentest code has a problem where we try to apply a border in our test when the web browser is not actually adding one. This happens when we do something like `border-top: 10px`. This will actually set the style of the border to `initial` which is just `none`, so nothing renders. This is causing at least 1 test to pass when it actually fails. I changed it so we ignore setting this value if the style is one of these values. I then re-ran the gentest code and excluded the now failing test (which gets fixed in my static stack). Reviewed By: NickGerleman Differential Revision: D51831754 fbshipit-source-id: a325e4a42b2d7cd6f19efc6cd5a2445574467fb7 --- gentest/fixtures/YGAbsolutePositionTest.html | 2 +- gentest/fixtures/YGFlexDirectionTest.html | 12 +-- gentest/gentest.js | 90 ++++++++++++------- .../facebook/yoga/YGAbsolutePositionTest.java | 9 +- .../generated/YGAbsolutePositionTest.test.ts | 10 +-- tests/generated/YGAbsolutePositionTest.cpp | 10 ++- 6 files changed, 81 insertions(+), 52 deletions(-) diff --git a/gentest/fixtures/YGAbsolutePositionTest.html b/gentest/fixtures/YGAbsolutePositionTest.html index 8ccdc14b..16bd6e39 100644 --- a/gentest/fixtures/YGAbsolutePositionTest.html +++ b/gentest/fixtures/YGAbsolutePositionTest.html @@ -93,7 +93,7 @@
-
+
diff --git a/gentest/fixtures/YGFlexDirectionTest.html b/gentest/fixtures/YGFlexDirectionTest.html index 716ca61d..70f7c067 100644 --- a/gentest/fixtures/YGFlexDirectionTest.html +++ b/gentest/fixtures/YGFlexDirectionTest.html @@ -307,7 +307,7 @@
-
+
@@ -315,7 +315,7 @@
-
+
@@ -323,7 +323,7 @@
-
+
@@ -331,7 +331,7 @@
-
+
@@ -339,7 +339,7 @@
-
+
@@ -347,7 +347,7 @@
-
+
diff --git a/gentest/gentest.js b/gentest/gentest.js index 8e053173..ea5b22e8 100755 --- a/gentest/gentest.js +++ b/gentest/gentest.js @@ -10,6 +10,8 @@ const DEFAULT_EXPERIMENTS = ['AbsolutePercentageAgainstPaddingEdge']; +const INVISIBLE_BORDER_STYLES = ['none', 'initial']; + window.onload = function () { checkDefaultValues(); @@ -432,48 +434,72 @@ function setupTestTree( ); break; case 'border-left-width': - if (genericNode.rawStyle.indexOf('border-start-width:') >= 0) { - e.YGNodeStyleSetBorder( - nodeName, - e.YGEdgeStart, - pointValue(e, node.style[style]), - ); - } else { - e.YGNodeStyleSetBorder( - nodeName, - e.YGEdgeLeft, - pointValue(e, node.style[style]), - ); + if ( + !INVISIBLE_BORDER_STYLES.includes( + node.declaredStyle['border-left-style'], + ) + ) { + if (genericNode.rawStyle.indexOf('border-start-width:') >= 0) { + e.YGNodeStyleSetBorder( + nodeName, + e.YGEdgeStart, + pointValue(e, node.style[style]), + ); + } else { + e.YGNodeStyleSetBorder( + nodeName, + e.YGEdgeLeft, + pointValue(e, node.style[style]), + ); + } } break; case 'border-top-width': - e.YGNodeStyleSetBorder( - nodeName, - e.YGEdgeTop, - pointValue(e, node.style[style]), - ); - break; - case 'border-right-width': - if (genericNode.rawStyle.indexOf('border-end-width:') >= 0) { + if ( + !INVISIBLE_BORDER_STYLES.includes( + node.declaredStyle['border-top-style'], + ) + ) { e.YGNodeStyleSetBorder( nodeName, - e.YGEdgeEnd, - pointValue(e, node.style[style]), - ); - } else { - e.YGNodeStyleSetBorder( - nodeName, - e.YGEdgeRight, + e.YGEdgeTop, pointValue(e, node.style[style]), ); } break; + case 'border-right-width': + if ( + !INVISIBLE_BORDER_STYLES.includes( + node.declaredStyle['border-right-style'], + ) + ) { + if (genericNode.rawStyle.indexOf('border-end-width:') >= 0) { + e.YGNodeStyleSetBorder( + nodeName, + e.YGEdgeEnd, + pointValue(e, node.style[style]), + ); + } else { + e.YGNodeStyleSetBorder( + nodeName, + e.YGEdgeRight, + pointValue(e, node.style[style]), + ); + } + } + break; case 'border-bottom-width': - e.YGNodeStyleSetBorder( - nodeName, - e.YGEdgeBottom, - pointValue(e, node.style[style]), - ); + if ( + !INVISIBLE_BORDER_STYLES.includes( + node.declaredStyle['border-bottom-style'], + ) + ) { + e.YGNodeStyleSetBorder( + nodeName, + e.YGEdgeBottom, + pointValue(e, node.style[style]), + ); + } break; case 'width': e.YGNodeStyleSetWidth(nodeName, pointValue(e, node.style[style])); diff --git a/java/tests/com/facebook/yoga/YGAbsolutePositionTest.java b/java/tests/com/facebook/yoga/YGAbsolutePositionTest.java index 9d802361..db06c1c0 100644 --- a/java/tests/com/facebook/yoga/YGAbsolutePositionTest.java +++ b/java/tests/com/facebook/yoga/YGAbsolutePositionTest.java @@ -1124,6 +1124,7 @@ public class YGAbsolutePositionTest { } @Test + @Ignore public void test_absolute_layout_percentage_height_based_on_padded_parent() { YogaConfig config = YogaConfigFactory.create(); config.setExperimentalFeatureEnabled(YogaExperimentalFeature.ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE, true); @@ -1149,9 +1150,9 @@ public class YGAbsolutePositionTest { assertEquals(100f, root.getLayoutHeight(), 0.0f); assertEquals(0f, root_child0.getLayoutX(), 0.0f); - assertEquals(10f, root_child0.getLayoutY(), 0.0f); + assertEquals(20f, root_child0.getLayoutY(), 0.0f); assertEquals(100f, root_child0.getLayoutWidth(), 0.0f); - assertEquals(50f, root_child0.getLayoutHeight(), 0.0f); + assertEquals(45f, root_child0.getLayoutHeight(), 0.0f); root.setDirection(YogaDirection.RTL); root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); @@ -1162,9 +1163,9 @@ public class YGAbsolutePositionTest { assertEquals(100f, root.getLayoutHeight(), 0.0f); assertEquals(0f, root_child0.getLayoutX(), 0.0f); - assertEquals(10f, root_child0.getLayoutY(), 0.0f); + assertEquals(20f, root_child0.getLayoutY(), 0.0f); assertEquals(100f, root_child0.getLayoutWidth(), 0.0f); - assertEquals(50f, root_child0.getLayoutHeight(), 0.0f); + assertEquals(45f, root_child0.getLayoutHeight(), 0.0f); } @Test diff --git a/javascript/tests/generated/YGAbsolutePositionTest.test.ts b/javascript/tests/generated/YGAbsolutePositionTest.test.ts index 808d2a49..2197819b 100644 --- a/javascript/tests/generated/YGAbsolutePositionTest.test.ts +++ b/javascript/tests/generated/YGAbsolutePositionTest.test.ts @@ -1255,7 +1255,7 @@ test('percent_absolute_position_infinite_height', () => { config.free(); } }); -test('absolute_layout_percentage_height_based_on_padded_parent', () => { +test.skip('absolute_layout_percentage_height_based_on_padded_parent', () => { const config = Yoga.Config.create(); let root; @@ -1282,9 +1282,9 @@ test('absolute_layout_percentage_height_based_on_padded_parent', () => { expect(root.getComputedHeight()).toBe(100); expect(root_child0.getComputedLeft()).toBe(0); - expect(root_child0.getComputedTop()).toBe(10); + expect(root_child0.getComputedTop()).toBe(20); expect(root_child0.getComputedWidth()).toBe(100); - expect(root_child0.getComputedHeight()).toBe(50); + expect(root_child0.getComputedHeight()).toBe(45); root.calculateLayout(undefined, undefined, Direction.RTL); @@ -1294,9 +1294,9 @@ test('absolute_layout_percentage_height_based_on_padded_parent', () => { expect(root.getComputedHeight()).toBe(100); expect(root_child0.getComputedLeft()).toBe(0); - expect(root_child0.getComputedTop()).toBe(10); + expect(root_child0.getComputedTop()).toBe(20); expect(root_child0.getComputedWidth()).toBe(100); - expect(root_child0.getComputedHeight()).toBe(50); + expect(root_child0.getComputedHeight()).toBe(45); } finally { if (typeof root !== 'undefined') { root.freeRecursive(); diff --git a/tests/generated/YGAbsolutePositionTest.cpp b/tests/generated/YGAbsolutePositionTest.cpp index 521e4699..f2fcdbcf 100644 --- a/tests/generated/YGAbsolutePositionTest.cpp +++ b/tests/generated/YGAbsolutePositionTest.cpp @@ -1132,6 +1132,8 @@ TEST(YogaTest, percent_absolute_position_infinite_height) { } TEST(YogaTest, absolute_layout_percentage_height_based_on_padded_parent) { + GTEST_SKIP(); + const YGConfigRef config = YGConfigNew(); YGConfigSetExperimentalFeatureEnabled(config, YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge, true); @@ -1155,9 +1157,9 @@ TEST(YogaTest, absolute_layout_percentage_height_based_on_padded_parent) { ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root)); ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0)); - ASSERT_FLOAT_EQ(10, YGNodeLayoutGetTop(root_child0)); + ASSERT_FLOAT_EQ(20, YGNodeLayoutGetTop(root_child0)); ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0)); - ASSERT_FLOAT_EQ(50, YGNodeLayoutGetHeight(root_child0)); + ASSERT_FLOAT_EQ(45, YGNodeLayoutGetHeight(root_child0)); YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL); @@ -1167,9 +1169,9 @@ TEST(YogaTest, absolute_layout_percentage_height_based_on_padded_parent) { ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root)); ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0)); - ASSERT_FLOAT_EQ(10, YGNodeLayoutGetTop(root_child0)); + ASSERT_FLOAT_EQ(20, YGNodeLayoutGetTop(root_child0)); ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0)); - ASSERT_FLOAT_EQ(50, YGNodeLayoutGetHeight(root_child0)); + ASSERT_FLOAT_EQ(45, YGNodeLayoutGetHeight(root_child0)); YGNodeFreeRecursive(root);