New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testshell_markdown_directoryvalue: failed #1753

Closed
mpranj opened this Issue Dec 22, 2017 · 8 comments

Comments

Projects
None yet
3 participants
@mpranj
Contributor

mpranj commented Dec 22, 2017

Steps to Reproduce the Problem

build & make run_all

Actual Result

110/155 Test #110: testshell_markdown_directoryvalue ........***Failed    0.83 sec
kdb mount config.file /examples/directoryvalue directoryvalue
kdb umount /examples/directoryvalue
kdb mount config.file /examples/directoryvalue directoryvalue
kdb set /examples/directoryvalue/harold 'Father of SpongeBob SquarePants'
Return value “5” does not match “0”
kdb set /examples/directoryvalue/harold/spongebob 'I am ready!'
Return value “5” does not match “0”
kdb set /examples/directoryvalue/patrick Star
Return value “5” does not match “0”
kdb set /examples/directoryvalue/patrick/#0 'Being grown-up is boring. Besides, I don’t get Jazz.'
Return value “5” does not match “0”
kdb ls /examples/directoryvalue
Return value “5” does not match “0”

ERROR - STDOUT:
“”
does not match
“user/examples/directoryvalue/harold⏎user/examples/directoryvalue/harold/spongebob⏎user/examples/directoryvalue/patrick⏎user/examples/directoryvalue/patrick/#0”

kdb get /examples/directoryvalue/harold
Return value “5” does not match “0”

ERROR - STDOUT:
“”
does not match
“Father of SpongeBob SquarePants”

kdb get /examples/directoryvalue/harold/spongebob
Return value “5” does not match “0”

ERROR - STDOUT:
“”
does not match
“I am ready!”

kdb get /examples/directoryvalue/patrick
Return value “5” does not match “0”

ERROR - STDOUT:
“”
does not match
“Star”

kdb get /examples/directoryvalue/patrick/#0
Return value “5” does not match “0”

ERROR - STDOUT:
“”
does not match
“Being grown-up is boring. Besides, I don’t get Jazz.”

kdb getmeta /examples/directoryvalue/patrick array
Return value “5” does not match “0”

ERROR - STDOUT:
“”
does not match
“#0”

kdb rm -r /examples/directoryvalue
Return value “5” does not match “0”
kdb umount /examples/directoryvalue
shell_recorder /tmp/tmp.x5ANYiDui7 RESULTS: 36 test(s) done 17 error(s).
📕
Protocol File: /tmp/elektraenv.NFWx9Dhxj

System Information

  • Elektra Version: master (9ba0271)
  • Fedora
@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Dec 22, 2017

Contributor

Thank you for reporting!

Contributor

markus2330 commented Dec 22, 2017

Thank you for reporting!

@sanssecours sanssecours removed their assignment Dec 22, 2017

@sanssecours

This comment has been minimized.

Show comment
Hide comment
@sanssecours

sanssecours Dec 22, 2017

Contributor

I just installed Fedora in a virtual machine. On my end testshell_markdown_directoryvalue works as expected, after I create /etc/kdb and transfer the ownership of the folder to the current user.

@mpranj 👋 Can you please clean your build directory and check if the issue persists?

Contributor

sanssecours commented Dec 22, 2017

I just installed Fedora in a virtual machine. On my end testshell_markdown_directoryvalue works as expected, after I create /etc/kdb and transfer the ownership of the folder to the current user.

@mpranj 👋 Can you please clean your build directory and check if the issue persists?

@mpranj

This comment has been minimized.

Show comment
Hide comment
@mpranj

mpranj Dec 22, 2017

Contributor

It appears that this was the only test that read from a corrupted ~/.config/config.file.
Sorry for the false alarm. This is why I don't like tests that do anything outside of the build directory.

Contributor

mpranj commented Dec 22, 2017

It appears that this was the only test that read from a corrupted ~/.config/config.file.
Sorry for the false alarm. This is why I don't like tests that do anything outside of the build directory.

@mpranj mpranj closed this Dec 22, 2017

@markus2330 markus2330 referenced this issue Dec 22, 2017

Open

Shell Recorder Problems #944

9 of 18 tasks complete

@sanssecours sanssecours self-assigned this Dec 22, 2017

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Dec 22, 2017

Contributor

This is why I don't like tests that do anything outside of the build directory.

Nobody likes them :-) You can run make run_nokdbtests to only run tests not interacting with KDB.

But @mpranj is right, the shell recorder should modify all mount commands to be more safe.

So for example, if we have:
sudo kdb mount config.file /examples/directoryvalue directoryvalue

it actually should mount with a command like:
kdb mount <unique-directory>/config.file tests/shell/<unique-id>/examples/directoryvalue directoryvalue

Btw. at least on github <br> and two spaces at EOL are quite different.

Contributor

markus2330 commented Dec 22, 2017

This is why I don't like tests that do anything outside of the build directory.

Nobody likes them :-) You can run make run_nokdbtests to only run tests not interacting with KDB.

But @mpranj is right, the shell recorder should modify all mount commands to be more safe.

So for example, if we have:
sudo kdb mount config.file /examples/directoryvalue directoryvalue

it actually should mount with a command like:
kdb mount <unique-directory>/config.file tests/shell/<unique-id>/examples/directoryvalue directoryvalue

Btw. at least on github <br> and two spaces at EOL are quite different.

@markus2330 markus2330 reopened this Dec 22, 2017

@sanssecours sanssecours removed their assignment Dec 22, 2017

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Dec 22, 2017

Contributor

Unfortunate that it is not possible to put smileys at " sanssecours removed their assignment"

Contributor

markus2330 commented Dec 22, 2017

Unfortunate that it is not possible to put smileys at " sanssecours removed their assignment"

@sanssecours

This comment has been minimized.

Show comment
Hide comment
@sanssecours

sanssecours Dec 22, 2017

Contributor

it actually should mount with a command like:
kdb mount <unique-directory>/config.file tests/shell/<unique-id>/examples/directoryvalue directoryvalue

I think that is a very bad idea. The Markdown Shell Recorder should execute commands as they are written down. Changing commands just

  • introduces new problems in Markdown Shell Recorder tests that are not actual problems,
  • masks problems that happen using the actual command, instead of the one the Markdown Shell Recorder executes

.

Contributor

sanssecours commented Dec 22, 2017

it actually should mount with a command like:
kdb mount <unique-directory>/config.file tests/shell/<unique-id>/examples/directoryvalue directoryvalue

I think that is a very bad idea. The Markdown Shell Recorder should execute commands as they are written down. Changing commands just

  • introduces new problems in Markdown Shell Recorder tests that are not actual problems,
  • masks problems that happen using the actual command, instead of the one the Markdown Shell Recorder executes

.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Dec 22, 2017

Contributor

introduces new problems in Markdown Shell Recorder tests that are not actual problems,

Afaik we never had problems due to changing a command to be more safe but we already had multiple problems for not changing commands. If we do not change them, they might be harmful, as in this case. The content of config.file could be important, the content of /etc/hosts nearly certainly is (for paths starting with / we already implemented the modification).

masks problems that happen using the actual command, instead of the one the Markdown Shell Recorder executes

We can show what we actually do. But humans are not good in guessing unique IDs. Computers are.

Contributor

markus2330 commented Dec 22, 2017

introduces new problems in Markdown Shell Recorder tests that are not actual problems,

Afaik we never had problems due to changing a command to be more safe but we already had multiple problems for not changing commands. If we do not change them, they might be harmful, as in this case. The content of config.file could be important, the content of /etc/hosts nearly certainly is (for paths starting with / we already implemented the modification).

masks problems that happen using the actual command, instead of the one the Markdown Shell Recorder executes

We can show what we actually do. But humans are not good in guessing unique IDs. Computers are.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Dec 23, 2017

Contributor

I agree with you that modifying Elektra paths might not be necessary. (It also really complicates the shell recorder because nearly every command would need to be rewritten.) We should, however, enforce for the mount that Elektra paths are within a well-defined, reserved set of Elektra paths (like /examples, /tests, and /tutorials) so that real configuration is never touched.

Contributor

markus2330 commented Dec 23, 2017

I agree with you that modifying Elektra paths might not be necessary. (It also really complicates the shell recorder because nearly every command would need to be rewritten.) We should, however, enforce for the mount that Elektra paths are within a well-defined, reserved set of Elektra paths (like /examples, /tests, and /tutorials) so that real configuration is never touched.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment