Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#1150 closed enhancement (fixed)

python test scripts generated by configure should be in _SCRIPTS

Reported by: jinmei Owned by: vorner
Priority: high Milestone: Sprint-20110830
Component: build system Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 2 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

In xfrout/tests, the test script is generated from .in at ./configure
time, but the generated script is specified as EXTRA_DIST:

PYTESTS = xfrout_test.py
EXTRA_DIST = $(PYTESTS)

(there's xfrout_test.py.in)

This is not good in that

  • we include the auto generated file in the tar ball (not necessarily harmful, but it's redundant)
  • when we edit .py.in, we need to run the top level ./configure again in order to generate the corresponding .py

Instead, we should do specify .py as noinst_SCRIPTS like this:

PYTESTS = xfrout_test.py
noinst_SCRIPTS = $(PYTESTS)
# no EXTRA_DIST

Then, gnu make can automatically regenerate .py (and only that .py)
when we update .py.in.

But that would cause a regression in tests for process.rename (in
lib/python/isc/util). So this task will include preventing the
regression.

Also, there may be other such test scripts. They should be identified
and fixed same way.

Subtickets

Change History (12)

comment:1 Changed 8 years ago by jinmei

  • Owner changed from jreed to UnAssigned
  • Status changed from new to assigned

comment:2 Changed 8 years ago by jinmei

#904 contains the difficult part of this task, so once it's merged
this task will be quite trivial. And I believe the result will be
very useful to improve productivity, so I'd strongly suggest including
this ticket in the next sprint.

To highlight the intent I'm going to upgrade the priority.

comment:3 Changed 8 years ago by jinmei

  • Priority changed from major to critical

comment:4 Changed 8 years ago by shane

  • Milestone changed from Next-Sprint-Proposed to Sprint-20110830

comment:5 Changed 8 years ago by vorner

  • Owner changed from UnAssigned to vorner
  • Status changed from assigned to accepted

comment:6 Changed 8 years ago by vorner

  • Owner changed from vorner to UnAssigned
  • Status changed from accepted to reviewing

It should be done. I found only 5 such scripts (I expected more, but find and grep seem confident about it) and one was already in noinst_SCRIPTS.

comment:7 Changed 8 years ago by stephen

  • Owner changed from UnAssigned to stephen

comment:8 Changed 8 years ago by stephen

  • Owner changed from stephen to vorner

Reviewed 43da3c6c1cc7cb5fcb1dbe2f983a53e883408d1b

OK apart from:

src/lib/python/isc/log/tests/Makefile.am
This file has:

PYTESTS_GEN = log_test.py
noinst_SCRIPTS = $(PYTESTS) log_console.py

The tests don't run as PYTESTS is not defined. Also, PYTESTS_GEN seems to imply that log_test.py is generated; in fact log_console.py is generated from a .in file, log_test.py is not.

comment:9 Changed 8 years ago by vorner

  • Owner changed from vorner to stephen

Ah, yes, you're completely right, I messed that file. It should be fixed now. As they end up in different directories during distcheck, they need to be handled separately (I wanted to use $(addprefix) to construct full paths and run the cycle only once, but it seems it's gmake specific).

Anyway, I noticed, when the .in is modified, it does regenerate the test script, but doesn't chmod +x it. Should we add it to the makefile somehow?

Thank you

comment:10 Changed 8 years ago by stephen

  • Owner changed from stephen to vorner

That seems to solve it, you can merge now.

Anyway, I noticed, when the .in is modified, it does regenerate the test script, but doesn't chmod +x it. Should we add it to the makefile somehow?

Yes, although I think this is something that needs to be done in configure.ac

comment:11 Changed 8 years ago by vorner

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

Well, it is generated by configure and set to executable the first time.

But if I touch foo_test.py.in and call make && make check, make regenerates foo_test.py from the .in, but does not set it executable any more. I don't think there's a reasonable way to say it should be in the configure.ac, so I put it into the makefile just before running it.

So, thanks, I merged it with this change.

comment:12 Changed 8 years ago by stephen

  • Estimated Difficulty changed from 0 to 2
Note: See TracTickets for help on using tickets.