Fix JSON output of targets --streaming --keep-going when there is an …

…error

Summary:
Right now, if the output of `stream_packages` for one of the packages produces only an error, it will add an unnecessary trailing separator, causing malformed JSON.

Instead, just add the separator if we have something else to show.

Reviewed By: zertosh

Differential Revision: D68892303

fbshipit-source-id: 5b957c60eb86e146a29593847691d3bb0688d090

app/buck2_server_commands/src/commands/targets/streaming.rs

@@ -115,12 +115,14 @@ pub(crate) async fn targets_streaming(
                             };
                             match targets {
                                 Ok((eval_result, targets, err)) => {
-                                    if let Some(err) = err {
-                                        show_err(&err.into());
-                                        formatter.separator(&mut res.stdout);
+                                    if let Some(ref err) = err {
+                                        show_err(err.into());
                                     }
                                     res.stats.success += 1;
                                     if imports {
+                                        if err.is_some() {
+                                            formatter.separator(&mut res.stdout);
+                                        }
                                         let eval_imports = eval_result.imports();
                                         formatter.imports(
                                             &eval_result.buildfile_path().path(),
@@ -135,7 +137,7 @@ pub(crate) async fn targets_streaming(
                                     }
                                     for (i, node) in targets.iter().enumerate() {
                                         res.stats.targets += 1;
-                                        if imports || i != 0 {
+                                        if err.is_some() || imports || i != 0 {
                                             formatter.separator(&mut res.stdout);
                                         }
                                         formatter.target(

tests/core/targets_command/test_targets_keep_going.py

@@ -75,12 +75,13 @@ async def test_streaming_keep_going_with_single_failure(buck: Buck) -> None:
         "//a:does_not_exist",
     ]
     result = await buck.targets(*targets, "--json", "--streaming", "--keep-going")
-    try:
-        # TODO(T213880451) we shouldn't emit malformed json on an error
-        json.loads(result.stdout)
-        raise AssertionError("Expected json to fail to parse")
-    except json.decoder.JSONDecodeError:
-        pass
+    xs = json.loads(result.stdout)
+    assert len(xs) == 1
+    assert xs[0]["buck.package"] == "root//a"
+    assert (
+        xs[0]["buck.error"]
+        == "Unknown targets `does_not_exist` from package `root//a`."
+    )
 
 
 @buck_test()
@@ -99,9 +100,20 @@ async def test_streaming_keep_going_with_single_failing_target_and_one_other_tar
         "--keep-going",
     )
 
-    try:
-        # TODO(T213880451) we shouldn't emit malformed json on an error
-        json.loads(result.stdout)
-        raise AssertionError("Expected json to fail to parse")
-    except json.decoder.JSONDecodeError:
-        pass
+    xs = json.loads(result.stdout)
+    assert len(xs) == 2
+
+    if "buck.error" in xs[0]:
+        good_target = xs[1]
+        bad_target = xs[0]
+    else:
+        good_target = xs[0]
+        bad_target = xs[1]
+
+    assert good_target["buck.type"] == "prelude//prelude.bzl:a_target"
+
+    assert bad_target["buck.package"] == "root//c"
+    assert (
+        bad_target["buck.error"]
+        == "Unknown targets `does_not_exist` from package `root//c`."
+    )