Run isolation tests if there is specs folder.#3
Conversation
funbringer
left a comment
funbringer
left a comment
There was a problem hiding this comment.
Thanks for a nice feature! Can't wait to merge this when you're finished with dealing with those nitpicks.
| isolation_args = [ | ||
| os.path.join(instance.work_dir, | ||
| "src/test/isolation/pg_isolation_regress"), | ||
| "--temp-instance=%s" % tmpdir, |
There was a problem hiding this comment.
Bikeshedding: I personally prefer format to %, since this is a recommended way of string formatting in 3.0+. Could you rewrite those pieces of code, please?
|
|
||
| self._regress_opts = None | ||
|
|
||
| def get_temp_config(self): |
There was a problem hiding this comment.
This function is a perfect candidate for being a property.
|
|
||
| def make(self, targets=None, options=None): | ||
| self.specs = None | ||
| specs_dir = os.path.join(work_dir, 'specs') |
There was a problem hiding this comment.
We probably shouldn't hardwire specs dir name. Maybe we could add a default-able flag to the pgxs command?
| "src/test/isolation/pg_isolation_regress"), | ||
| "--temp-instance=%s" % tmpdir, | ||
| "--inputdir=.", | ||
| "--outputdir=output_iso", |
There was a problem hiding this comment.
Needless contraction -- output_isolation?
| if target in ('check', 'installcheck') and self.specs: | ||
| print(Style.green('\n$ make isolationcheck')) | ||
|
|
||
| tmpdir = os.path.join(self.work_dir, "tmp_check_iso") |
There was a problem hiding this comment.
Maybe we should use mkdtemp to make sure that dir name is unique? Otherwise it could result in name clash when used in parallel builds.
2 participants
No description provided.