Use a context manager to restore state of functional test environment

[Ryan J Ollos]

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?

[Jun Omae]

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​.

[Ryan J Ollos]

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.

[Jun Omae]

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.

[Ryan J Ollos]

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.

[Ryan J Ollos]

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