Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

shell recorder test cases fail if spec mounted #1975

Closed
markus2330 opened this issue May 11, 2018 · 12 comments · Fixed by #1985
Closed

shell recorder test cases fail if spec mounted #1975

markus2330 opened this issue May 11, 2018 · 12 comments · Fixed by #1985
Labels
Milestone

Comments

@markus2330
Copy link
Contributor

Steps to Reproduce the Problem

(~e is Elektra source, ~b is Elektra build dir)

kdb mount ~e/current/src/tools/gen/tests/lift.ini spec/lift ni
cd ~b
make run_all

Expected Result

No error when executing test cases.

Actual Result

...
The command kdb import terminated unsuccessfully with the info:
Invalid Keyname: keyname needs to start with /, spec/, proc/, dir/, user/ or system/ or maybe you tried to change a key that is already in a KeySet.
Please report the issue at https://issues.libelektra.org/
fatal: Could not import spec config
...

The following tests FAILED:
         38 - testshell_markdown_base64 (Failed)
         40 - testshell_markdown_blockresolver (Failed)
         42 - testshell_markdown_boolean (Failed)
         43 - testshell_markdown_cachefilter (Failed)
         46 - testshell_markdown_camel (Failed)
         49 - testshell_markdown_conditionals (Failed)
         50 - testshell_markdown_constants (Failed)
         55 - testshell_markdown_csvstorage (Failed)
         58 - testshell_markdown_dini (Failed)
         60 - testshell_markdown_directoryvalue (Failed)
         63 - testshell_markdown_enum (Failed)
         71 - testshell_markdown_hosts (Failed)
         74 - testshell_markdown_ini (Failed)
         77 - testshell_markdown_ipaddr (Failed)
         80 - testshell_markdown_line (Failed)
         84 - testshell_markdown_mathcheck (Failed)
         87 - testshell_markdown_mini (Failed)
         89 - testshell_markdown_mozprefs (Failed)
         90 - testshell_markdown_multifile (Failed)
         95 - testshell_markdown_range (Failed)
         99 - testshell_markdown_tcl (Failed)
        101 - testshell_markdown_template (Failed)
        102 - testshell_markdown_type (Failed)
        105 - testshell_markdown_xerces (Failed)
        109 - testshell_markdown_yajl (Failed)
        111 - testshell_markdown_yamlcpp (Failed)
        135 - testshell_db_changes (Failed)
        136 - testshell_host (Failed)
        137 - testshell_listtest (Failed)
        138 - testshell_mathcheck (Failed)
        139 - testshell_profiletest (Failed)
        140 - testshell_script (Failed)
        141 - testshell_selftest (Failed)
        142 - testshell_replay_ls (Failed)
        143 - testshell_markdown_msr_syntax (Failed)
        144 - testshell_markdown_ini_crash_test (Failed)
        145 - testshell_markdown_kdb-global-umount (Failed)
        146 - testshell_markdown_readme_msr (Failed)
        147 - testshell_markdown_issue_template (Failed)
        148 - testshell_markdown_tutorial_cascading (Failed)
        149 - testshell_markdown_kdb-complete (Failed)
        150 - testshell_markdown_kdb-ls (Failed)
        151 - testshell_markdown_tutorial_validation (Failed)

System Information

  • Elektra Version: master

Further Log Files and Output

@markus2330 markus2330 added this to the 0.8.23 milestone May 11, 2018
@markus2330 markus2330 added the bug label May 11, 2018
@sanssecours sanssecours changed the title shell recorder test cases fail if spec mounted shell test cases fail if spec mounted May 12, 2018
@sanssecours sanssecours changed the title shell test cases fail if spec mounted shell recorder test cases fail if spec mounted May 12, 2018
sanssecours added a commit to sanssecours/elektra that referenced this issue May 12, 2018
This commit tries to reproduce the problem described in issue ElektraInitiative#1975.
sanssecours added a commit to sanssecours/elektra that referenced this issue May 12, 2018
This commit tries to reproduce the problem described in issue ElektraInitiative#1975.

# Conflicts:
#	.travis.yml
sanssecours added a commit to sanssecours/elektra that referenced this issue May 12, 2018
This commit tries to reproduce the problem described in issue ElektraInitiative#1975.

# Conflicts:
#	.travis.yml
sanssecours added a commit to sanssecours/elektra that referenced this issue May 12, 2018
This commit tries to reproduce the problem described in issue ElektraInitiative#1975.

# Conflicts:
#	.travis.yml
@sanssecours
Copy link
Member

I tried to reproduce the issue using simple kdb commands. The code below shows a “minimal” example:

kdb mount "$PWD/src/tools/gen/tests/lift.ini" spec/lift ni
kdb export spec > /tmp/spec.dump
kdb rm -r spec
kdb import spec < /tmp/spec.dump

.

@markus2330
Copy link
Contributor Author

markus2330 commented May 12, 2018

Could you reproduce it with these commands?

There should be no difference between mounting or importing a file (in the resulting KDB).

@sanssecours
Copy link
Member

Could you reproduce it with these commands?

Yes.

There should be no difference between mounting or importing a file.

So, the commands from my comment above should work without problems?

@markus2330
Copy link
Contributor Author

Thank you for your help!

I misunderstood what you wanted to do. So above commands (after mount) should reproduce what the shell recorder does? (while it is failing?)

If so, yes, in your commands I now get the same error like the shell recorder.

I did not get the same error before, because user/sw/kdb/current/format said ni (and thus the export was in ni, and there was no error at the import).

lift.ini (in the source) is removed when using the dump plugin.
But it looks like it has the same content with the ni plugin (after export/import).

It'll need further investigation.

@sanssecours
Copy link
Member

So above commands (after mount) should reproduce what the shell recorder does? (while it is failing?)

Correct.

@markus2330
Copy link
Contributor Author

It seems like there are cascading keys in the export where the import fails.

This is related to #51

Why do we export and import spec in the first place? There are many plugins that do not completely restore the formatting, so we should not do such things unless absolutely necessary.

Do we have MountpointRoot as / somewhere? It should be always below /tests/ see #1887.

@sanssecours
Copy link
Member

Why do we export and import spec in the first place?

That’s because we want to keep the configuration just like it was before. One method to do that is to export the configuration before a test and re-import it afterwards.

There are many plugins that do not completely restore the formatting, so we should not do such things unless absolutely necessary.

We always use dump to export and import the config. So in theory there should be no problem with this approach.

Do we have MountpointRoot as / somewhere?

That depends on the test, but many Markdown Shell Recorder test use something like /examples/plugin.

It should be always below /tests/ see #1887.

I know 😊.

@markus2330
Copy link
Contributor Author

That’s because we want to keep the configuration just like it was before. One method to do that is to export the configuration before a test and re-import it afterwards.

Yes, it is perfectly okay if this is done for the parts where tests actually write to.

So in theory there should be no problem with this approach.

Unfortunately there is: it depends on which plugins are used locally. Not all plugins can keep formatting and order. A complete and safe rewrite of all configuration settings is of course a goal but we cannot know what people do locally (they can write their own broken plugins).

That depends on the test, but many Markdown Shell Recorder test use something like /examples/plugin.

That should be fine and not effect spec/lift. So why is spec exported and imported then?

@sanssecours
Copy link
Member

So why is spec exported and imported then?

That is the case, since we export and import the whole configuration regardless of the content of a Shell Recorder test. By the way, we do the same in run_all.sh.

@markus2330
Copy link
Contributor Author

Afaik, this only exports the whole configuration but does not import it again, unless IMPORTCONFIG is set. I did not find a place where it is set except in the shell recorder, which causes the trouble as reported here.

What do you think about #1985?

It makes a lot of sense to have a backup of the configuration, I do not want to change that.

@sanssecours
Copy link
Member

What do you think about #1985?

I guess this update should fix the problem. Looks good to me.

@markus2330
Copy link
Contributor Author

There is also a different problem:

kdb export uses ksCut to cut all keys below parent key (spec in this case). @tom-wa defined that /something is below spec/, which causes the cascading keys to be included in the export and the import to fail. #51 would fix it because it would separate namespaces.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants