This is an archive of the discontinued Mercurial Phabricator instance.

tests: look for CRLF on Windows
Needs RevisionPublic

Authored by indygreg on Mar 29 2020, 9:30 PM.

Download Raw Diff

Details

Reviewers

durin42
marmoute
baymax

Group Reviewers

hg-reviewers

Summary

The ui.editor code calls util.tonativeeol() to normalize line endings
to the platform native format. So, editor files may have CRLF
in them. For some reason, just this one specific test case was
failing on Python 3 on Windows. (The output before this change
was ...\r (esc), but only on the first line of output. I suspect
the test harness output parser was getting confused by the specific
byte sequences in this output or something.

I wanted to hack around this by filtering output via sed to remove
the CR. But check-code says this practice is apparently banned.
So I just made the test conditional.

Diff Detail

Repository

rHG Mercurial

Branch

default

Lint

No Linters Available

Unit

No Unit Test Coverage

Event Timeline

indygreg created this revision.Mar 29 2020, 9:30 PM

Herald added a reviewer: durin42. · View Herald TranscriptMar 29 2020, 9:30 PM

Herald added a reviewer: hg-reviewers. · View Herald Transcript

Herald added a subscriber: mercurial-devel. · View Herald Transcript

indygreg added a child revision: D8346: tests: force newlines to LF in tinyproxy.py.Mar 29 2020, 9:30 PM

The test harness *should* match existing \n output as a fallback, which got me to wondering if it was the (esc) at the end screwing it up. I tried this patch:

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -2065,6 +2065,8 @@ class TTest(Test):
             if PYTHON3:
                 el = el[:-7].decode('unicode_escape') + '\n'
                 el = el.encode('utf-8')
+                log('el: %s' % el)
+                log(' l: %s' % l)
             else:
                 el = el[:-7].decode('string-escape') + '\n'
         if el == l or os.name == 'nt' and el[:-1] + b'\r\n' == l:

... and got this output:

$ time py -3 run-tests.py --local test-histedit-arguments.t#abortflag --view bcompare
running 1 tests using 1 parallel processes
el: b'pick 3d3ea1f3a10b 5 1234567890123456789012345678901234567890123456789012345\xc3\xa3\xc2\x81\xc2\x82...\n'
 l: b'pick 3d3ea1f3a10b 5 1234567890123456789012345678901234567890123456789012345\xe3\x81\x82...\r\n'

So I'm not sure where the bytes are being lost. If this doesn't suggest a larger problem, I've got no problem with this patch as written. But I wonder if there's some stdio that needs to be patched up, because I thought I saw that pattern in some failing py3 tests at one point.

@mharbison72 @indygreg, IIRC Matt is saying this is the wrong fix. What's the status of this?

marmoute mentioned this in D8346: tests: force newlines to LF in tinyproxy.py.Apr 17 2020, 2:34 PM

marmoute mentioned this in D8348: tests: force newlines to LF in inline Python script.

In D8345#126157, @marmoute wrote:

@mharbison72 @indygreg, IIRC Matt is saying this is the wrong fix. What's the status of this?

I'm not sure that it's *wrong*, I'm just wondering if there's an encoding and/or stdio issue lurking here somewhere.

marmoute accepted this revision.Apr 17 2020, 4:00 PM

There seems to have been no activities on this Diff for the past 3 Months.

By policy, we are automatically moving it out of the need-review state.

Please, move it back to need-review without hesitation if this diff should still be discussed.

:baymax:need-review-idle:

This revision now requires changes to proceed.Jul 31 2020, 1:56 PM

Herald added a subscriber: mercurial-patches. · View Herald TranscriptJul 31 2020, 1:56 PM

Revision Contents
Changeset List

			Path	Packages
M			tests/test-histedit-arguments.t (23 lines)

Commit	Parents	Author	Summary	Date
c45605166431	3ef7069a4676	Gregory Szorc		Mar 29 2020, 6:02 PM

Status	Author	Revision
Needs Revision	indygreg	D8348 tests: force newlines to LF in inline Python script
Needs Revision	indygreg	D8347 encoding: use special dictionary type for env variables on Windows
Needs Revision	indygreg	D8346 tests: force newlines to LF in tinyproxy.py
Needs Revision	indygreg	D8345 tests: look for CRLF on Windows
Closed	indygreg	D8344 extensions: refactor function for obtaining disabled extension help
Closed	indygreg	D8343 tests: perform grep manually in test-doctest.py
Closed	indygreg	D8342 tests: prevent printing \r to stdout
Closed	indygreg	D8341 tests: force \n newlines when writing to sys.stdout
Abandoned	indygreg	D8340 dispatch: force \n for newlines on sys.std* streams (BC)
Closed	indygreg	D8339 dispatch: force \n for newlines on sys.std* streams (BC)
Closed	indygreg	D8338 hook: move stdio redirection to context manager

Diff 20918

tests/test-histedit-arguments.t

	> # 2 x 4 = 8 columns, but 3 x 4 = 12 bytes			> # 2 x 4 = 8 columns, but 3 x 4 = 12 bytes
	> fp.write(u'\u3042\u3044\u3046\u3048'.encode('utf-8'))			> fp.write(u'\u3042\u3044\u3046\u3048'.encode('utf-8'))
	>			>
	> fp.close()			> fp.close()
	> EOF			> EOF
	$ echo xx >> x			$ echo xx >> x
	$ hg --encoding utf-8 commit --logfile logfile			$ hg --encoding utf-8 commit --logfile logfile

				Editor code calls util.tonativeeol, which only inserts CRLF on Python 3 on Windows.
				#if py3 windows
				$ HGEDITOR=cat hg --encoding utf-8 histedit tip
				pick 3d3ea1f3a10b 5 1234567890123456789012345678901234567890123456789012345\xe3\x81\x82...\r (esc)
				\r (esc)
				# Edit history between 3d3ea1f3a10b and 3d3ea1f3a10b\r (esc)
				#\r (esc)
				# Commits are listed from least to most recent\r (esc)
				#\r (esc)
				# You can reorder changesets by reordering the lines\r (esc)
				#\r (esc)
				# Commands:\r (esc)
				#\r (esc)
				# e, edit = use commit, but stop for amending\r (esc)
				# m, mess = edit commit message without changing commit content\r (esc)
				# p, pick = use commit\r (esc)
				# b, base = checkout changeset and apply further changesets from there\r (esc)
				# d, drop = remove commit from history\r (esc)
				# f, fold = use commit, but combine it with the one above\r (esc)
				# r, roll = like fold, but discard this commit's description and date\r (esc)
				#\r (esc)
				#else
	$ HGEDITOR=cat hg --encoding utf-8 histedit tip			$ HGEDITOR=cat hg --encoding utf-8 histedit tip
	pick 3d3ea1f3a10b 5 1234567890123456789012345678901234567890123456789012345\xe3\x81\x82... (esc)			pick 3d3ea1f3a10b 5 1234567890123456789012345678901234567890123456789012345\xe3\x81\x82... (esc)

	# Edit history between 3d3ea1f3a10b and 3d3ea1f3a10b			# Edit history between 3d3ea1f3a10b and 3d3ea1f3a10b
	#			#
	# Commits are listed from least to most recent			# Commits are listed from least to most recent
	#			#
	# You can reorder changesets by reordering the lines			# You can reorder changesets by reordering the lines
	#			#
	# Commands:			# Commands:
	#			#
	# e, edit = use commit, but stop for amending			# e, edit = use commit, but stop for amending
	# m, mess = edit commit message without changing commit content			# m, mess = edit commit message without changing commit content
	# p, pick = use commit			# p, pick = use commit
	# b, base = checkout changeset and apply further changesets from there			# b, base = checkout changeset and apply further changesets from there
	# d, drop = remove commit from history			# d, drop = remove commit from history
	# f, fold = use commit, but combine it with the one above			# f, fold = use commit, but combine it with the one above
	# r, roll = like fold, but discard this commit's description and date			# r, roll = like fold, but discard this commit's description and date
	#			#
				#endif

	Test --continue with --keep			Test --continue with --keep

	$ hg strip -q -r . --config extensions.strip=			$ hg strip -q -r . --config extensions.strip=
	$ hg histedit '.^' -q --keep --commands - << EOF			$ hg histedit '.^' -q --keep --commands - << EOF
	> edit eb57da33312f 2 three			> edit eb57da33312f 2 three
	> pick f3cfcca30c44 4 x			> pick f3cfcca30c44 4 x
	> EOF			> EOF

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	20918			Mar 29 2020, 9:30 PM	★	★