Page MenuHomePhabricator

Robustify update_test_checks.py to non-autogened tests, and add a mode to skip non-autogenerated ones
ClosedPublic

Authored by reames on Aug 1 2019, 3:12 PM.

Details

Summary

Intended use case is:
./utils/update_test_checks.py test/Transform/PassDir/* --update-only
(i.e. rapidly be able to see changes in autogened filed, before handing non-autogened tests individually)

Diff Detail

Repository
rL LLVM

Event Timeline

reames created this revision.Aug 1 2019, 3:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2019, 3:12 PM
RKSimon added a subscriber: RKSimon.

Could this functionality be added to all the update*py scripts?

MaskRay added inline comments.Aug 2 2019, 3:35 AM
utils/update_test_checks.py
65 ↗(On Diff #212912)

Prefer single quotes.

92 ↗(On Diff #212912)

'autogenerated'

113 ↗(On Diff #212912)

'...........{}'.format(l) is usually preferred over print('WARNING: Skipping unparseable RUN line: %s' % (l,), file=sys.stderr) (note comma after ll)

The comma makes it a one-element tuple. (l) is the same as l.

reames updated this revision to Diff 213089.Aug 2 2019, 10:58 AM
reames marked an inline comment as done.
reames marked 3 inline comments as done.

Could this functionality be added to all the update*py scripts?

Could be reasonable done, but given the divergence already existing, would need to be hand implemented for each. I'm not volunteering.

utils/update_test_checks.py
113 ↗(On Diff #212912)

I'd prefer to follow the style of all the surrounding code which uses this construct.

p.s. The common vs non-common form appear to evaluate exactly the same. (Based on the output of a test only)

xbolva00 added a subscriber: xbolva00.EditedAug 2 2019, 11:19 AM

Thanks, useful!

I'm not volunteering.

You already did all coding work, copy (move to common) and test it shouldn’t be so hard. Anyway, after you land this, I could port your work to other update scripts.

xbolva00 added inline comments.Aug 2 2019, 11:29 AM
utils/update_test_checks.py
65 ↗(On Diff #212912)

Can you add short option?

MaskRay accepted this revision.Aug 4 2019, 9:36 AM
MaskRay added inline comments.
utils/update_test_checks.py
113 ↗(On Diff #212912)

Alternatively, 'WARNING: Skipping unparseable RUN line: ' + l

%s can mean str, int, list ([1,2,3]) and many other strings that can be converted to a str via str().

93 ↗(On Diff #213089)

"Skipping test which isn't autogenerated: " + test

112 ↗(On Diff #213089)

'|'

This revision is now accepted and ready to land.Aug 4 2019, 9:36 AM
This revision was automatically updated to reflect the committed changes.
reames marked an inline comment as done.