Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#12910 closed (fixed)

xgettext now required to run tests

Reported by: kmtracey Owned by: jezdez
Component: Uncategorized Version: 1.1
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Attempting to run tests on a Windows machine with no xgettext package install now dies a horrible death. The failure to find an xgettext executable causes the whole test run to just quit:

Installed 62 object(s) from 5 fixture(s)
......................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................Error: errors happened while running xgettext on __init__.py
'xgettext' is not recognized as an internal or external command,
operable program or batch file.


C:\u\kmt\django\trunk\tests>

I think we need to somehow figure out if xgettext is available and if not, skip running tests that require it.

Attachments (1)

12910.1.diff (9.3 KB) - added by jezdez 5 years ago.
Patch to only perform extraction tests if xgettext is found in PATH.

Download all attachments as: .zip

Change History (9)

comment:1 follow-up: Changed 5 years ago by jezdez

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Hm, does that only occur for Django's tests or for app tests as well?

comment:2 in reply to: ↑ 1 ; follow-up: Changed 5 years ago by ramiro

Replying to jezdez:

Hm, does that only occur for Django's tests or for app tests as well?

I suspect it is just the Django test suite with our new makemessages regression test. I can reproduce this by just running it (it seems there are other test failures when xgettext.exe does exist too).

And I suspect it could be related to this:

Summary: The old os.popen3 simply ignored the condition of a missing xgettext (or msgmerge or msgfmt) and we used that to our advantage (creating an empty .po file). Now subprocess.Popen() raises an OSError exception instead.

I had seen this and made a mental note about testing this under win32, but forgot to do it later.

Problem was I couldn't get Popen() to raise that OSError under Linux when trying to execute a non-existing external command (e.g. Popen('foo', shell=True, stdout=PIPE, stderr=PIPE, close_fds=False, universal_newlines=True).communicate())

comment:3 Changed 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

comment:4 in reply to: ↑ 2 Changed 5 years ago by jezdez

Replying to ramiro:

Replying to jezdez:

Hm, does that only occur for Django's tests or for app tests as well?

I suspect it is just the Django test suite with our new makemessages regression test. I can reproduce this by just running it (it seems there are other test failures when xgettext.exe does exist too).

Russ and me we thinking about making the makemessages tests optional just like we do with YAML tests for example, e.g. by a conditional import in the makemessages/tests.py if a call to xgettext succeeds.

And I suspect it could be related to this:

Summary: The old os.popen3 simply ignored the condition of a missing xgettext (or msgmerge or msgfmt) and we used that to our advantage (creating an empty .po file). Now subprocess.Popen() raises an OSError exception instead.

I had seen this and made a mental note about testing this under win32, but forgot to do it later.

Problem was I couldn't get Popen() to raise that OSError under Linux when trying to execute a non-existing external command (e.g. Popen('foo', shell=True, stdout=PIPE, stderr=PIPE, close_fds=False, universal_newlines=True).communicate())

Oddly enough Popen() will only raise the OSError for me if I pass it shell=False.

Changed 5 years ago by jezdez

Patch to only perform extraction tests if xgettext is found in PATH.

comment:5 Changed 5 years ago by jezdez

  • Owner changed from nobody to jezdez
  • Status changed from new to assigned

comment:6 Changed 5 years ago by jezdez

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [12475]) Fixed #12910 - Only test extracting translation strings if xgettext can be found on PATH.

comment:7 Changed 5 years ago by jezdez

(In [12589]) [1.1.X] Fixed #12910 - Only test extracting translation strings if xgettext can be found on PATH.

Backport from r12475.

comment:8 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top