Page MenuHomePhabricator

[lit] Print full googletest commad line
ClosedPublic

Authored by vitalybuka on May 20 2021, 9:52 PM.

Diff Detail

Event Timeline

vitalybuka created this revision.May 20 2021, 9:52 PM
vitalybuka requested review of this revision.May 20 2021, 9:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2021, 9:52 PM

The patch seems reasonable. I have a minor nit about the use of the % operator but other than that it seems fine.

llvm/utils/lit/lit/formats/googletest.py
125

Minor nit. This patch is mixing the old python string interpolation (% operator) and the format function.

Given that Python 3.6 is the minimum maybe we should just use format strings in both places e.g.

header = f"Script:\n--\n{' '.join(cmd)}\n--\n"
...
return (lit.test.TIMEOUT, f"{header}Reached timeout of {litConfig.maxIndividualTestTime} seconds")

upgrade formating

vitalybuka marked an inline comment as done.May 20 2021, 11:56 PM

Add UNRESOLVED test

rnk added a comment.May 21 2021, 8:17 AM

I see test failures on Windows that appear to be related to CRLF handling. I think the CHECK-NEXT checks aren't working because something is doubling up the newlines in the test output. That seems unrelated, so I suggest dropping the -NEXTs as necessary to get it past the presubmit testing.

vitalybuka planned changes to this revision.May 21 2021, 12:04 PM

it's just rebase so far, windows is not fixed
it does not reproduce for me so I will wait for another pre-merge

llvm/utils/lit/lit/formats/googletest.py
140

I assume we don't need it here

fix test on Windows

Issue is in the path separator.
lines like FAIL: googletest-upstream-format :: [[PATH:[Dd]ummy[Ss]ub[Dd]ir/]][[FILE:OneTest\.py]]/[[TEST:FirstTest\.subTestB]]
always have /
and command line is \

Reuse vars in tests

"x64 windows passed" now

rnk accepted this revision.May 21 2021, 2:48 PM

Classic.

lgtm

This revision is now accepted and ready to land.May 21 2021, 2:48 PM
This revision was landed with ongoing or failed builds.May 21 2021, 4:12 PM
This revision was automatically updated to reflect the committed changes.