Add unit tests for pkg/ovs/openflow #4221

hongliangl · 2022-09-13T03:34:54Z

Signed-off-by: Hongliang Liu lhongliang@vmware.com

codecov · 2022-09-13T03:59:01Z

Codecov Report

Merging #4221 (815f8b6) into main (1c0ec50) will decrease coverage by 3.98%.
The diff coverage is n/a.

❗ Current head 815f8b6 differs from pull request most recent head 045a711. Consider uploading reports for the commit 045a711 to get more accurate results

@@            Coverage Diff             @@
##             main    #4221      +/-   ##
==========================================
- Coverage   73.77%   69.80%   -3.98%     
==========================================
  Files         410      402       -8     
  Lines       60966    58384    -2582     
==========================================
- Hits        44978    40754    -4224     
- Misses      13082    14862    +1780     
+ Partials     2906     2768     -138

Flag	Coverage Δ		*Carryforward flag
e2e-tests	`38.37% <ø> (+0.02%)`	⬆️
integration-tests	`34.62% <ø> (+1.22%)`	⬆️	Carriedforward from 1d29ae3
kind-e2e-tests	`46.72% <ø> (-3.22%)`	⬇️	Carriedforward from 1d29ae3
unit-tests	`60.30% <ø> (-4.58%)`	⬇️	Carriedforward from 1d29ae3

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files	Coverage Δ
pkg/ovs/openflow/ofctrl_packetout.go	`99.16% <ø> (+15.36%)`	⬆️
pkg/ovs/ovsctl/interface.go	`16.66% <ø> (ø)`

... and 171 files with indirect coverage changes

pkg/ovs/openflow/ofctrl_action_test.go

hongliangl · 2022-09-30T10:21:45Z

This PR depends on #4258.

pkg/ovs/openflow/ofctrl_action_test.go

wenyingd

LGTM

pkg/agent/openflow/pipeline.go

pkg/ovs/openflow/ofctrl_action_test.go

pkg/ovs/openflow/ofctrl_builder_test.go

luolanzone

LGTM overall, a few nits.

pkg/ovs/openflow/testing/mock_openflow.go

luolanzone · 2023-06-19T08:47:35Z

pkg/ovs/openflow/utils.go

@@ -874,7 +874,12 @@ func nxActionLearnToString(action openflow15.Action) string {
 			if spec.SrcValue != nil {
 				srcValueStr := strings.TrimLeft(fmt.Sprintf("%x", spec.SrcValue), "0")
 				if isMatch {
-					dstFieldStr := getFieldNameString(spec.DstField.Field.Class, spec.DstField.Field.Field, spec.DstField.Ofs, nBits, true, false)
+					var dstFieldStr string
+					if spec.DstField.Field.Class == openflow15.OXM_CLASS_NXM_1 {


Can you explain a little bit why this change is needed? A new class is introduced after openflow15?

In the OpenFlow learn flow action, the registers in OpenFlow 1.5 (OXM_CLASS_NXM_1) are capitalized when used as an output.

pkg/ovs/openflow/ofctrl_packetout_test.go

luolanzone · 2023-06-19T09:32:36Z

@tnqn, @wenyingd please help to take a look for this unit test PR. Thanks.

wenyingd

Overall LGTM, one general comment is I saw many cases are almost the same except for the data value, maybe no need to add such duplicated cases?

pkg/agent/openflow/pipeline.go

wenyingd · 2023-06-19T09:53:13Z

pkg/ovs/openflow/ofctrl_builder_test.go

+					Value: &openflow15.ByteArrayField{Data: []byte{0x12, 0x34, 0x56}, Length: 3},
+				},
+			},
+			{


I don't get the differences among the three cases except for the data. Are they all needed?

Just to test different values. Besides, this can be also to test if the match string is as expected. For example, the first case should be xxreg0=0x123456, not xxreg0=0x00123456

pkg/ovs/openflow/ofctrl_builder_test.go

hongliangl · 2023-06-30T01:05:08Z

/test-all

pkg/ovs/openflow/ofctrl_action_test.go

pkg/ovs/openflow/ofctrl_builder_test.go

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

luolanzone

LGTM overall, a few nits for error check.

pkg/ovs/openflow/ofctrl_action_test.go

luolanzone · 2023-09-25T03:28:23Z

@tnqn @wenyingd please take a look as well, it would be better if we can close this in release 1.14. Thanks.

luolanzone · 2023-10-10T06:08:26Z

/test-all

@tnqn @wenyingd could you take a look? Thanks.

tnqn

LGTM

hongliangl · 2023-10-16T06:27:19Z

Thanks for merging and reviewing this PR @tnqn @luolanzone @wenyingd

hongliangl force-pushed the 20220906-ovs-ut branch from d8ab2d7 to a5a902f Compare September 13, 2022 03:37

hongliangl requested a review from wenyingd September 13, 2022 05:43

hongliangl mentioned this pull request Sep 13, 2022

Improve unit test coverage #4142

Open

37 tasks

hongliangl marked this pull request as draft September 14, 2022 10:21

hongliangl force-pushed the 20220906-ovs-ut branch from a5a902f to 6b3d255 Compare September 22, 2022 12:42

hongliangl marked this pull request as ready for review September 22, 2022 12:42

hongliangl requested a review from tnqn September 22, 2022 12:42

hongliangl force-pushed the 20220906-ovs-ut branch 2 times, most recently from c0af2c5 to 4274e52 Compare September 28, 2022 03:46

wenyingd reviewed Sep 29, 2022

View reviewed changes

hongliangl force-pushed the 20220906-ovs-ut branch 2 times, most recently from 7f12440 to d928d9e Compare September 30, 2022 10:12

hongliangl requested a review from wenyingd October 13, 2022 00:23

luolanzone added this to the Antrea v1.10 release milestone Oct 27, 2022

hongliangl force-pushed the 20220906-ovs-ut branch 2 times, most recently from c2f254f to 73b3334 Compare November 10, 2022 11:16

hongliangl commented Nov 16, 2022

View reviewed changes

pkg/ovs/openflow/ofctrl_action_test.go Outdated Show resolved Hide resolved

hongliangl force-pushed the 20220906-ovs-ut branch 4 times, most recently from f4feb52 to 8dea3e1 Compare November 21, 2022 01:05

hongliangl force-pushed the 20220906-ovs-ut branch 3 times, most recently from ff315c8 to 6e9d460 Compare December 16, 2022 00:07

wenyingd previously approved these changes Dec 16, 2022

View reviewed changes

hongliangl dismissed wenyingd’s stale review via 295ab33 December 16, 2022 09:56

hongliangl force-pushed the 20220906-ovs-ut branch 2 times, most recently from 295ab33 to 5344de4 Compare December 21, 2022 08:59

hongliangl requested a review from luolanzone June 16, 2023 02:57

hongliangl force-pushed the 20220906-ovs-ut branch 3 times, most recently from 0c7d665 to 97dccca Compare June 16, 2023 07:27

wenyingd reviewed Jun 16, 2023

View reviewed changes

hongliangl force-pushed the 20220906-ovs-ut branch from 97dccca to 9f4687b Compare June 19, 2023 02:02

hongliangl requested a review from wenyingd June 19, 2023 02:02

hongliangl force-pushed the 20220906-ovs-ut branch from 9f4687b to 866b88d Compare June 19, 2023 02:24

luolanzone reviewed Jun 19, 2023

View reviewed changes

wenyingd reviewed Jun 19, 2023

View reviewed changes

hongliangl force-pushed the 20220906-ovs-ut branch from 866b88d to 045a711 Compare June 20, 2023 08:59

hongliangl requested review from wenyingd and luolanzone June 20, 2023 09:00

hongliangl force-pushed the 20220906-ovs-ut branch from 045a711 to 55ff6b1 Compare June 29, 2023 03:18

tnqn reviewed Jul 3, 2023

View reviewed changes

hongliangl force-pushed the 20220906-ovs-ut branch from 55ff6b1 to e1c95fb Compare July 10, 2023 01:55

Add unit tests for pkg/ovs/openflow

25b496b

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

hongliangl force-pushed the 20220906-ovs-ut branch from e1c95fb to 25b496b Compare July 12, 2023 04:50

hongliangl requested a review from tnqn July 21, 2023 02:27

luolanzone modified the milestones: Antrea v1.13 release, Antrea v1.14 release Jul 21, 2023

luolanzone reviewed Sep 25, 2023

View reviewed changes

pkg/ovs/openflow/ofctrl_action_test.go Show resolved Hide resolved

pkg/ovs/openflow/ofctrl_action_test.go Show resolved Hide resolved

tnqn approved these changes Oct 16, 2023

View reviewed changes

tnqn merged commit dc0ca2d into antrea-io:main Oct 16, 2023
45 of 48 checks passed

hongliangl deleted the 20220906-ovs-ut branch October 16, 2023 06:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unit tests for pkg/ovs/openflow #4221

Add unit tests for pkg/ovs/openflow #4221

hongliangl commented Sep 13, 2022 •

edited

Loading

codecov bot commented Sep 13, 2022 •

edited

Loading

hongliangl commented Sep 30, 2022

wenyingd left a comment

luolanzone left a comment

luolanzone Jun 19, 2023

hongliangl Jun 20, 2023

luolanzone commented Jun 19, 2023

wenyingd left a comment

wenyingd Jun 19, 2023

hongliangl Jun 20, 2023

hongliangl commented Jun 30, 2023

luolanzone left a comment

luolanzone commented Sep 25, 2023

luolanzone commented Oct 10, 2023

tnqn left a comment

hongliangl commented Oct 16, 2023

Add unit tests for pkg/ovs/openflow #4221

Add unit tests for pkg/ovs/openflow #4221

Conversation

hongliangl commented Sep 13, 2022 • edited Loading

codecov bot commented Sep 13, 2022 • edited Loading

Codecov Report

hongliangl commented Sep 30, 2022

wenyingd left a comment

Choose a reason for hiding this comment

luolanzone left a comment

Choose a reason for hiding this comment

luolanzone Jun 19, 2023

Choose a reason for hiding this comment

hongliangl Jun 20, 2023

Choose a reason for hiding this comment

luolanzone commented Jun 19, 2023

wenyingd left a comment

Choose a reason for hiding this comment

wenyingd Jun 19, 2023

Choose a reason for hiding this comment

hongliangl Jun 20, 2023

Choose a reason for hiding this comment

hongliangl commented Jun 30, 2023

luolanzone left a comment

Choose a reason for hiding this comment

luolanzone commented Sep 25, 2023

luolanzone commented Oct 10, 2023

tnqn left a comment

Choose a reason for hiding this comment

hongliangl commented Oct 16, 2023

hongliangl commented Sep 13, 2022 •

edited

Loading

codecov bot commented Sep 13, 2022 •

edited

Loading