1391123 - ./mach try fuzzy should add a newline at the end of try_task_config.json

Status: RESOLVED FIXED

(Testing :: General, enhancement)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Looking at https://hg.mozilla.org/try/rev/d2539729043c396ee050207160bd5cdfd1121375 it complains about no newline at the end of the file. It should be pretty easy to add a newline and have it stop complaining about that.

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: Bug 1391123 - Add a newline to the end of try_task_config.json

Review commit: https://reviewboard.mozilla.org/r/180536/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180536/

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This seems to work. Is there a test that would need updated anywhere?

Comment 3

[Gregory Szorc [:gps]]

Comment on attachment 8908918 [details]
Bug 1391123 - Add a newline to the end of try_task_config.json

https://reviewboard.mozilla.org/r/180536/#review185672

AFAIK there are no tests that need to be updated for this.

::: tools/tryselect/vcs.py:88
(Diff revision 1)
>          with open(config, 'w') as fh:
>              try_task_config = {'tasks': sorted(labels)}
>              if templates:
>                  try_task_config['templates'] = templates
>  
> -            json.dump(try_task_config, fh, indent=2, separators=(',', ':'))
> +            json_text = json.dumps(try_task_config, indent=2, separators=(',', ':'))

We should continue to use ``json.dump`` here. Just add an ``fh.write('\n')`` after it to add the newline.

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8908918 [details]
Bug 1391123 - Add a newline to the end of try_task_config.json

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/180536/diff/1-2/

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8908918 [details]
Bug 1391123 - Add a newline to the end of try_task_config.json

https://reviewboard.mozilla.org/r/180536/#review185676

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8908918 [details]
Bug 1391123 - Add a newline to the end of try_task_config.json

https://reviewboard.mozilla.org/r/180536/#review185678

Oh, I do see there are "cram" tests for `mach try`. I even reviewed them, heh. `mach cramtest tools/tryselect`. Although they fail for me locally :/

You might want to run this by Try via `mach try fuzzy cram`.

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8908918 [details]
Bug 1391123 - Add a newline to the end of try_task_config.json

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/180536/diff/2-3/

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Updated the tests so they pass. I'm not sure why I had to add the whitespace to the newlines, though... Does that seem right to you?

Comment 9

[Gregory Szorc [:gps]]

The whitespace is needed to preserve the added newline. I reckon somewhere we are doing a print(), which will print the newline from the JSON plus a newline itself. (print() emits a trailing newline unless you tell it not to.)

I wouldn't worry too much about it. Let's land this.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6835fc2e493b
Add a newline to the end of try_task_config.json r=gps

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6835fc2e493b