web-platform-tests / results-collection

Other
41 stars 46 forks source link

Add a flag to the upload script to make timestamps optional #593

Closed Hexcles closed 6 years ago

Hexcles commented 6 years ago

Description

The main purpose of this PR is to make timestamps (namely, time_start & time_end optional). These two timestamps are optional fields in the report (see docs for results receiver), and certain infra doesn't have these two fields in their reports, in which case they can use this flag to safely omit these two fields (cc @thejohnjansen ).

This is added behind a new flag (i.e. the timestamps are still required by default) because I don't want to change the default behaviour, and the two fields should be present if reports are produced by wptrunner.

Changes

This PR also fixes some existing tests and removes a bunch of unused imports.

Hexcles commented 6 years ago

@jugglinmike I added a comment to say that the new flag, also the only optional flag, is for an external user. Besides, I tweaked the docstring to say that format has to meet the requirements of results receiver.

And regarding --no-timestamps, I think that's a bit too strict. "Optional" is more consistent with the requirement in the results receiver.

jugglinmike commented 6 years ago

And regarding --no-timestamps, I think that's a bit too strict.

I'm not familiar with the use case. Does the intended audience need to upload some files with timestamps and some files without timestamps?

"Optional" is more consistent with the requirement in the results receiver.

That may be so, but it's not opt-in in the results receiver.

Hexcles commented 6 years ago

@jugglinmike Hi Mike, I totally forgot this PR... I just rebased the PR and changed the flag from --optional-timestamps to --no-timestamps as you suggested. Can you take another look?

jugglinmike commented 6 years ago

Squashed and merged to master at the following commits:

Thanks, Robert!