is_not_rw_storage broken

[markus2330]

As @kodebach reported in #2419:

Yes, but that is broken, because is_not_rw_storage doesn't work. Not even dump is recognized as a storage plugin.

Fix is_not_rw_storage in include_common.sh.in tests/shell/check_export.sh so we detect problems like this in future.

• fix is_not_rw_storage
• fix testscr_check_import

[markus2330]

@kodebach Can you elaborate what exactly is broken in is_not_rw_storage?

[kodebach]

I just looked at the output of testscr_check_export (called with ctest -R testscr_check_export --verbose inside the build directory):

108: Check if script tests the correct version
108: base64 not a read-write storage
108: boolean not a read-write storage
108: c not a read-write storage
[...]
108: dump not a read-write storage
[...]

The test prints one line for each plugin that was compiled and then exits successfully. (Should probably fail if no read-write storage plugin was found, because at least one is required for Elektra to function properly)

Which has to correspond to these lines from tests/shell/check_export.sh:

	if is_not_rw_storage; then
		echo "$PLUGIN not a read-write storage"
		continue
	fi

So is_not_rw_storage always returns true.

[markus2330]

Thank you for reporting!

I agree it should fail if no plugins can be found.

[kodebach]

This issue also applies to testscr_check_import.

[llukask]

The problem with is_not_rw_storage is that it checks if the output of kdb info $PLUGIN provides is equal to storage, but for a plugin to be a storage plugin the output only has to contain the word storage.

When I fixed this in my fork all of the tests that use is_not_rw_storage (that is check_export, check_import, check_get_set and check_kdb_internal_suite) started to fail.

Most of the check_export and check_import tests fail because the necessary files in tests/shell/shell don't exist, but some fail for other reasons: the mmapstorage and mmapstorage_crc plugins fail because of permission stuff and the tcl plugin check_export tests fail because the output is incorrect. Those tests produce a lot of output so I might have missed something. I didn't look at check_get_set and check_kdb_internal_suite yet.

[kodebach]

Most of the check_export and check_import tests fail because the necessary files in tests/shell/shell don't exist

If you uncomment this line:

libelektra/tests/shell/generate_data.sh

Line 4 in 4fff878

exit 0

you can run the testcase testscr_generate_data to generate the files in tests/shell/shell. Some plugins however shouldn't be tested because they are not normal read-write-storage plugins (e.g. c, cpptemplate, specload), so you should first exclude the ones that shouldn't run.

For mmapstoarge to work the tests cannot use stdin/stdout (see #2401). DON'T try to fix the problem by running the test with sudo, this breaks /dev/stdout (see here).

[llukask]

In the process of fixing this a few problems occured, which is why i had to exclude some plugins (tcl, mini, simpleini) from the tests that probably should not be excluded.

The problems are all very similar, they all boil down to the same sequence of commands:

kdb set user/test hello
kdb export user/test $PLUGIN | kdb import user/test_other $PLUGIN

With the mini and tcl plugins this produces an error because the key name is empty (e.g. just =hello in ini).

The simpleini seems to not read anything from stdin and does nothing if you import = hello. The dini plugins seem to not read anything from stdin but can deal with importing = hello. Meaning the following works for dini (and for simpleini if the key name is not empty),

kdb set user/test hello
kdb export user/test $PLUGIN > test.ini
kdb import user/test_other $PLUGIN test.ini

[kodebach]

You could also try this:

kdb set user/test hello
kdb export user/test $PLUGIN test.ini
kdb import user/test_other $PLUGIN test.ini

Some plugins might have problems with writing to stdout as well as reading from stdin, because they need to read the file out of order.

[llukask]

Yes, I am doing that for dini

[markus2330]

(Assigned as already agreed in #2595)

[markus2330]

@kodebach is it still feasible that you solve this issue?

[kodebach]

I'll look at #2595 and see what needs to be done..

[mpranj]

If this was not completely resolved by #2595 please reopen.

[mpranj]

Seems like this failed on master now: https://build.libelektra.org/blue/organizations/jenkins/libelektra/detail/master/242/pipeline/661

[kodebach]

Only the version with the ini plugin as default storage failed. I don't the problem is with is_not_rw_storage, it probably is just another bug in ini. Maybe we should just disable this build job, since it will be removed soon anyway.

[mpranj]

Thank you for looking into it. Yes, ini is about to be removed very soon. If I remove it now we will have a merge conflict with #3491. We can't cherry-pick since the commit also changes other files. @markus2330 is it ok if you resolve the conflict in your PR?

[markus2330]

Thank you both 🎆

@mpranj yes, it would be appreciated if you do fixes in the PR!

Having a fixed master has of course the highest priority 🚀

[markus2330]

Everything should be fixed now. @kodebach Can we close this issue?

[kodebach]

AFAIK everything is fixed