-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add run-tests.php --context [n]
option.
#5968
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
be77f82
to
40d33b2
Compare
Example output can be seen at https://travis-ci.org/github/php/php-src/jobs/717434379 . Lines starting with The line numbers will take up more columns for additions or removals in all places if either the expected or actual output take up more than 999 lines. This PR was created before the PR to colorize diff output was merged.
The expectation of DateTime_diff-spring-type3-type2.phpt
|
Mentioned in php#5965 (comment) This PR proposes 3 lines of context so the impact can be seen in tests. Other `diff` programs show around 3 lines of context. (This helps indicate exactly which position a test should be updated to add a new expected line at) Use the mapping for choosing order to display diffs Properly include context in cases where the expected output had more lines than the actual output, e.g. ``` --FILE-- A A1 A C NEARBY --EXPECTF-- A B A1 B A B A B NEARBY ```
40d33b2
to
c75a2f4
Compare
c75a2f4
to
a366f70
Compare
LGTM and can indeed help while debugging tests failures on CI. Just FYI Nikita is currently on vacation so if you want his input, you will need to wait. |
Mentioned in #5965 (comment)
This PR proposes 3 lines of context so the impact can be seen in tests.
Other
diff
programs show around 3 lines of context.(This helps indicate exactly which position a test should be updated
to add a new expected line at)
Are there reasons to continue not printing lines of context for test failures?
The lack of context seems inconvenient when investigating CI failures or when the line order wasn't obvious.