It appears that Milo is removing double <br> in the output. This makes some of the output much harder to read. I've attached a screenshot, you can also compare it manuall. Compare https://luci-milo.appspot.com/buildbot/tryserver.chromium.linux/linux_chromium_rel_ng/415680 and https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/415680
Perhaps Milo needs a test to avoid regressions
I don't think a test would've helped here, since it's parsing the <br> correctly, but due to my poor understanding of how divs work it wasn't rendered.
a test that has an empty step_text line and an nbsp in expectation files would catch this
It would, if the rendering behavior was known in advance
you are right, we didn't know the behavior that we should have provided in advance let's add a test so that we don't introduce regressions
The following revision refers to this bug: https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/26d81493a96488be4ec8b6bc00fb3129de58eb15 commit 26d81493a96488be4ec8b6bc00fb3129de58eb15 Author: hinoka <hinoka@google.com> Date: Mon Mar 27 16:26:40 2017 Milo: Fix empty lines in step text BUG= 704809 Review-Url: https://codereview.chromium.org/2768913005 [modify] https://crrev.com/26d81493a96488be4ec8b6bc00fb3129de58eb15/milo/api/proto/buildbot.pb.go [modify] https://crrev.com/26d81493a96488be4ec8b6bc00fb3129de58eb15/milo/api/proto/buildinfo.pb.go [modify] https://crrev.com/26d81493a96488be4ec8b6bc00fb3129de58eb15/milo/api/proto/pb.discovery.go [add] https://crrev.com/26d81493a96488be4ec8b6bc00fb3129de58eb15/milo/appengine/buildbot/expectations/newline.1234.build.json [modify] https://crrev.com/26d81493a96488be4ec8b6bc00fb3129de58eb15/milo/appengine/buildbot/html_data.go [add] https://crrev.com/26d81493a96488be4ec8b6bc00fb3129de58eb15/milo/appengine/buildbot/testdata/newline.1234.json [add] https://crrev.com/26d81493a96488be4ec8b6bc00fb3129de58eb15/milo/appengine/frontend/expectations/buildbot.build-Debug_page-_newline_1234.html [modify] https://crrev.com/26d81493a96488be4ec8b6bc00fb3129de58eb15/milo/appengine/frontend/templates/pages/build.html [modify] https://crrev.com/26d81493a96488be4ec8b6bc00fb3129de58eb15/milo/appengine/logdog/internal/stream.pb.go
Issue 670172 has been merged into this issue.
Comment 1 by no...@chromium.org
, Mar 24 2017Owner: hinoka@chromium.org
Status: Assigned (was: Untriaged)