Skip to content

Conversation

@benoit-nexthop
Copy link
Contributor

@benoit-nexthop benoit-nexthop commented Oct 15, 2025

Description

Fix CmdShowInterfaceTrafficTestFixture so it passes when stdout is a terminal.

Motivation

The code was mishandling when to add colors to the output in a way that
made the test fail when stdout was a terminal:

$ /var/FBOSS/tmp_bld_dir/build/fboss/fboss2_cmd_test 
--gtest_filter=CmdShowInterfaceTrafficTestFixture.printOutput
Note: Google Test filter = 
CmdShowInterfaceTrafficTestFixture.printOutput [==========] Running 1 
test from 1 test suite. [----------] Global test environment set-up. 
[----------] 1 test from CmdShowInterfaceTrafficTestFixture [ RUN      ]
CmdShowInterfaceTrafficTestFixture.printOutput ERRORS 1 interfaces, 
watch for any incrementing counters: 
/var/FBOSS/fboss/fboss/cli/fboss2/test/CmdShowInterfaceTrafficTest.cpp:186:
Failure Expected equality of these values:
  expectedOutput
    Which is: "ERRORS 1 interfaces, watch for any incrementing counters:\n Interface Name  Peer         Status  FCS  Align  Symbol  Rx   Runts  Giants  Tx  \n---------------------------------------------------------------------------------------------\n eth2/1/1        fsw002.p001  up      0    0      0       100  0      0       100 \n\n Interface Name  Peer         Intvl  InMbps    InPct  InKpps  OutMbps    OutPct  OutKpps \n---------------------------------------------------------------------------------------------------\n eth1/1/1        fsw001.p001  0:60   9876.54   9.88%  6.00    79012.35   79.01%  15.00   \n eth2/1/1        fsw002.p001  0:60   9876.54   4.94%  6.00    79012.35   39.51%  15.00   \n eth3/1/1        fsw003.p001  0:60   9876.54   2.47%  6.00    79012.35   19.75%  15.00   \n Total           --           --     29629.63  4.23%  18.00   237037.04  33.86%  45.00   \n\n"
  output
    Which is: " Interface Name  Peer         Status  FCS  Align  Symbol  Rx   Runts  Giants  Tx  \n---------------------------------------------------------------------------------------------\n eth2/1/1        fsw002.p001  up      0    0      0       100  0      0       100 \n\n Interface Name  Peer         Intvl  InMbps    InPct  InKpps  OutMbps    OutPct  OutKpps \n---------------------------------------------------------------------------------------------------\n eth1/1/1        fsw001.p001  0:60   9876.54   9.88%  6.00    79012.35   79.01%  15.00   \n eth2/1/1        fsw002.p001  0:60   9876.54   4.94%  6.00    79012.35   39.51%  15.00   \n eth3/1/1        fsw003.p001  0:60   9876.54   2.47%  6.00    79012.35   19.75%  15.00   \n Total           --           --     29629.63  4.23%  18.00   237037.04  33.86%  45.00   \n\n"
With diff:
@@ -1,3 @@
-ERRORS 1 interfaces, watch for any incrementing counters:
  Interface Name  Peer         Status  FCS  Align  Symbol  Rx   Runts  Giants  Tx  
 ---------------------------------------------------------------------------------------------

[  FAILED  ] CmdShowInterfaceTrafficTestFixture.printOutput (6 ms)
[----------] 1 test from CmdShowInterfaceTrafficTestFixture (6 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (6 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CmdShowInterfaceTrafficTestFixture.printOutput

 1 FAILED TEST

Whereas the test would pass if stdout wasn’t a terminal:

$ /var/FBOSS/tmp_bld_dir/build/fboss/fboss2_cmd_test --gtest_filter=CmdShowInterfaceTrafficTestFixture.printOutput | cat
Note: Google Test filter = CmdShowInterfaceTrafficTestFixture.printOutput
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from CmdShowInterfaceTrafficTestFixture
[ RUN      ] CmdShowInterfaceTrafficTestFixture.printOutput
[       OK ] CmdShowInterfaceTrafficTestFixture.printOutput (6 ms)
[----------] 1 test from CmdShowInterfaceTrafficTestFixture (6 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (6 ms total)
[  PASSED  ] 1 test.

Also when using EXPECT_EQ() the expected value must come first. For
multi-line strings it makes a difference because the diff is printed in
reverse otherwise.

Test Plan

Existing unit tests pass.

@meta-cla meta-cla bot added the CLA Signed label Oct 15, 2025
@benoit-nexthop benoit-nexthop force-pushed the fix-show-interface-traffic-test branch 2 times, most recently from 537afde to c09dc93 Compare October 16, 2025 20:48
@meta-codesync
Copy link

meta-codesync bot commented Oct 27, 2025

@joseph5wu has imported this pull request. If you are a Meta employee, you can view this in D85605968.

The code was mishandling when to add colors to the output in a way that 
made the test fail when stdout was a terminal:

```
$ /var/FBOSS/tmp_bld_dir/build/fboss/fboss2_cmd_test 
--gtest_filter=CmdShowInterfaceTrafficTestFixture.printOutput
Note: Google Test filter = 
CmdShowInterfaceTrafficTestFixture.printOutput [==========] Running 1 
test from 1 test suite. [----------] Global test environment set-up. 
[----------] 1 test from CmdShowInterfaceTrafficTestFixture [ RUN      ]
CmdShowInterfaceTrafficTestFixture.printOutput ERRORS 1 interfaces, 
watch for any incrementing counters: 
/var/FBOSS/fboss/fboss/cli/fboss2/test/CmdShowInterfaceTrafficTest.cpp:186:
Failure Expected equality of these values:
  expectedOutput
    Which is: "ERRORS 1 interfaces, watch for any incrementing counters:\n Interface Name  Peer         Status  FCS  Align  Symbol  Rx   Runts  Giants  Tx  \n---------------------------------------------------------------------------------------------\n eth2/1/1        fsw002.p001  up      0    0      0       100  0      0       100 \n\n Interface Name  Peer         Intvl  InMbps    InPct  InKpps  OutMbps    OutPct  OutKpps \n---------------------------------------------------------------------------------------------------\n eth1/1/1        fsw001.p001  0:60   9876.54   9.88%  6.00    79012.35   79.01%  15.00   \n eth2/1/1        fsw002.p001  0:60   9876.54   4.94%  6.00    79012.35   39.51%  15.00   \n eth3/1/1        fsw003.p001  0:60   9876.54   2.47%  6.00    79012.35   19.75%  15.00   \n Total           --           --     29629.63  4.23%  18.00   237037.04  33.86%  45.00   \n\n"
  output
    Which is: " Interface Name  Peer         Status  FCS  Align  Symbol  Rx   Runts  Giants  Tx  \n---------------------------------------------------------------------------------------------\n eth2/1/1        fsw002.p001  up      0    0      0       100  0      0       100 \n\n Interface Name  Peer         Intvl  InMbps    InPct  InKpps  OutMbps    OutPct  OutKpps \n---------------------------------------------------------------------------------------------------\n eth1/1/1        fsw001.p001  0:60   9876.54   9.88%  6.00    79012.35   79.01%  15.00   \n eth2/1/1        fsw002.p001  0:60   9876.54   4.94%  6.00    79012.35   39.51%  15.00   \n eth3/1/1        fsw003.p001  0:60   9876.54   2.47%  6.00    79012.35   19.75%  15.00   \n Total           --           --     29629.63  4.23%  18.00   237037.04  33.86%  45.00   \n\n"
With diff:
@@ -1,3 @@
-ERRORS 1 interfaces, watch for any incrementing counters:
  Interface Name  Peer         Status  FCS  Align  Symbol  Rx   Runts  Giants  Tx  
 ---------------------------------------------------------------------------------------------

[  FAILED  ] CmdShowInterfaceTrafficTestFixture.printOutput (6 ms)
[----------] 1 test from CmdShowInterfaceTrafficTestFixture (6 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (6 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CmdShowInterfaceTrafficTestFixture.printOutput

 1 FAILED TEST
```
Whereas the test would pass if `stdout` wasn’t a terminal:
```
$ /var/FBOSS/tmp_bld_dir/build/fboss/fboss2_cmd_test --gtest_filter=CmdShowInterfaceTrafficTestFixture.printOutput | cat
Note: Google Test filter = CmdShowInterfaceTrafficTestFixture.printOutput
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from CmdShowInterfaceTrafficTestFixture
[ RUN      ] CmdShowInterfaceTrafficTestFixture.printOutput
[       OK ] CmdShowInterfaceTrafficTestFixture.printOutput (6 ms)
[----------] 1 test from CmdShowInterfaceTrafficTestFixture (6 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (6 ms total)
[  PASSED  ] 1 test.
```

Also when using `EXPECT_EQ()` the expected value must come first. For 
multi-line strings it makes a difference because the diff is printed in 
reverse otherwise.
@benoit-nexthop benoit-nexthop force-pushed the fix-show-interface-traffic-test branch from c09dc93 to 9a38601 Compare October 27, 2025 23:33
@facebook-github-bot
Copy link
Contributor

@benoit-nexthop has updated the pull request. You must reimport the pull request before landing.

@meta-codesync
Copy link

meta-codesync bot commented Oct 29, 2025

@joseph5wu merged this pull request in 3868f77.

@benoit-nexthop benoit-nexthop deleted the fix-show-interface-traffic-test branch October 30, 2025 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants