Edgewall Software
Modify

Opened 6 years ago

Closed 4 years ago

#11320 closed enhancement (wontfix)

Use a context manager to restore state of functional test environment

Reported by: Ryan J Ollos Owned by:
Priority: normal Milestone:
Component: general Version:
Severity: normal Keywords: functional-tests
Cc: Branch:
Release Notes:
API Changes:

Description

There is a repeated pattern of try / finally in the functional test cases. I started looking at whether we could use a context manager to restore state at the end of each test case. The simplest thing I could think to do for an initial implementation is make a copy of the trac.ini file at the start of test execution, and then restore and reparse it at the end of the test case.

With the changes that can be found in log:rjollos.git:t11320, I executed RegressionTestTicket6747 100 times before and after making the changes. The execution time was 410 s before the change, and 265 s after. I'm guessing that it's faster because we aren't calling Configuration.save.

Next I added some code to log back in as admin at the end of the test case. It doesn't seem to have measurably affected the execution time on repeated (n = 100) execution of RegressionTestTicket11028.

If we were to take this further, additional things could be done on entry / exit:

  • Copy / restore the permission table in the database. Another possibility would be to store all of the calls to grant_perm and revoke_perm and undo the changes.
  • FunctionalTestEnvironment.enable_authz_permpolicy / disable_authz_permpolicy.
  • Copy / restore all tables?

Attachments (0)

Change History (6)

in reply to:  description ; comment:1 by Jun Omae, 6 years ago

I'm guessing that it's faster because we aren't calling Configuration.save.

Not sure. In [cc071de6/rjollos.git], you have removed self._testenv.restart() in finally blocks, however, the EnvironmentRestored doesn't call the restart.

Also, the modified time of trac.ini is restored by copy2(). I don't think we should restore it. Configuration.parse_if_needed() in test tracd webserver sometimes may not parse it on next request. See branches/1.0-stable/trac/config.py@12083:267-268#L262.

in reply to:  1 ; comment:2 by Ryan J Ollos, 6 years ago

Replying to jomae:

I'm guessing that it's faster because we aren't calling Configuration.save.

Not sure. In [cc071de6/rjollos.git], you have removed self._testenv.restart() in finally blocks, however, the EnvironmentRestored doesn't call the restart.

Yeah, I think you are right, restart is where the majority of the time is spent. When I put a testcase._testenv.restart() in my contextmanager, the execution time returns to ~395 seconds. Adding a env.config.save() brings the execution time to 405 seconds.

Also, the modified time of trac.ini is restored by copy2(). I don't think we should restore it. Configuration.parse_if_needed() in test tracd webserver sometimes may not parse it on next request. See branches/1.0-stable/trac/config.py@12083:267-268#L262.

That makes sense, I'll use copy() instead. Note that I also force a reparse, just to be sure, but it doesn't appear to be necessary if copy is used unless we run into the low resolution filesystem issue most recently discussed in #11069. Speaking of which, maybe we could just use a parse_if_needed(True) rather than time.sleep(1)? I will give that a try and make a comment on #11069.

in reply to:  2 comment:3 by Jun Omae, 6 years ago

Replying to rjollos:

… Speaking of which, maybe we could just use a parse_if_needed(True) rather than time.sleep(1)? I will give that a try and make a comment on #11069.

If functional tests, FunctionalTestSuite.setUp() launches tracd web server as a new process. When conf/trac.ini is changed, the tracd also must parse it. The instance of Environment in test suite parses it by parse_if_needed(force=True), however, the instance of Environment in tracd doesn't parse it by the method. We have no way to parse in tracd other than changing the modified time of trac.ini.

But, if using ext3 file system, parse_if_needed(force=False) sometimes cannot detect the changes of trac.ini caused by low time resolution.

comment:4 by Ryan J Ollos, 6 years ago

Thanks, it seems rather obvious now that you've explained it!

For this ticket, I need to implement a way to "undo" the permission changes, and at that point I'd consider integrating this work. I'll probably leave it for after 1.0.2 though, unless I get some inspiration soon.

comment:5 by Jun Omae, 5 years ago

Keywords: functional-tests added; functional tests removed

comment:6 by Ryan J Ollos, 4 years ago

Milestone: next-stable-1.0.x
Resolution: wontfix
Status: newclosed

I probably won't pursue this, focusing instead on #11988.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The ticket will remain with no owner.
The resolution will be deleted.
to The owner will be changed from (none) to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.