Conversation
regress/expected/tablespace.out
Outdated
| \! mkdir /tmp/tblspc_testts | ||
| mkdir: cannot create directory ‘/tmp/tblspc_testts’: File exists | ||
| \! chmod 0700 /tmp/tblspc_testts | ||
| CREATE TABLESPACE testts LOCATION '/tmp/tblspc_testts'; |
There was a problem hiding this comment.
I don't think this is something that can be solved so easily. If any, I would use mkdir -p so that an existing directory is not a problem. The problem I see there is that many ubuntu distributions delete /tmp on boot, so you leave whatever service in a broken state.
I don't think that this operation belongs to the test suite. It is an integration test. the github workflow creates it ok; on dev machine it should be left to something else.
Alternatively, still not robust, but maybe we could drop the namespace at the end of the test run, and remove the directory?
There was a problem hiding this comment.
I totally agree with you... just adding some more comments in the tests to make developers life easier.
In TimescaleDB we have a wrapper for pg_regress that take care of tablespaces creation amoung other things: https://github.com/timescale/timescaledb/blob/main/test/pg_regress.sh#L156-L186
There was a problem hiding this comment.
I agree with @dvarrazzo that using -p makes this easier to handle, and it can't fail.
Dropping the tablespace (aka: cleanup after the test) is also a good idea.
There was a problem hiding this comment.
@dvarrazzo I guess now it is in a good shape!
There was a problem hiding this comment.
Ok with the changes to the CI and the instuction in the tests about creating the tablespace 👍
Can't remember if we talked about that - sorry, I'm a bit out of context: what is the reason for removing the
replace(... ' TABLESPACE pg_default', ''),
rather than including the tablespace in the output?
There was a problem hiding this comment.
I also don't remember why I've added it... hehehe... maybe at that time I found some flakyness but let me check again!!!
There was a problem hiding this comment.
If it's a matter of different postgres versions emitting different output, I assume you know that every test file can have more than one output file (looks like we actually have 5 of them for the tablespace tests... I guess some can be dropped!)
There was a problem hiding this comment.
If it's a matter of different postgres versions emitting different output, I assume you know that every test file can have more than one output file (looks like we actually have 5 of them for the tablespace tests... I guess some can be dropped!)
You're correct... in this commit just updated all the test output files with the new comments and the cleanup but dind't changed the content... not sure what files we can remove because we're supporting pg10 to pg16. Maybe a following PR to check it properly should be better.
fabriziomello
force-pushed
the
fix_failing_tablespace_test
branch
7 times, most recently
from
3dbb3f1 to
7824ac7
Compare
fabriziomello
force-pushed
the
fix_failing_tablespace_test
branch
2 times, most recently
from
5655450 to
b34e6de
Compare
fabriziomello
force-pushed
the
fix_failing_tablespace_test
branch
from
b34e6de to
6bfe771
Compare
fabriziomello
force-pushed
the
fix_failing_tablespace_test
branch
from
6bfe771 to
827c975
Compare
3 participants
No description provided.