Skip to content
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

Add tutorial for running tests with Docker #2768

Conversation

Projects
None yet
4 participants
@oleksandr-shabelnyk
Copy link
Contributor

commented Jun 10, 2019

@markus2330 Please review this tutorial. Unfortunately, since a I have a lot of pending changes for another issue, I could not create a separate PR just for this tutorial, so I had to do it from my different account, not from @alexsaber . I hope it is OK for you.

Basics

  • [x ] Short descriptions should be in the release notes (added as entry in
    doc/news/_preparation_next_release.md which contains _(my name)_)
    Please always add something to the the release notes.
  • Longer descriptions should be in documentation or in design decisions.
  • Describe details of how you changed the code in commit messages
    (first line should have module: short statement syntax)
  • References to issues, e.g. close #X, should be in the commit messages.

Checklist

Check relevant points but please do not remove entries.
For docu fixes, spell checking, and similar none of these points below
need to be checked.

  • I added unit tests
  • I ran all tests locally and everything went fine
  • affected documentation is fixed
  • I added code comments, logging, and assertions (see Coding Guidelines)
  • meta data is updated (e.g. README.md of plugins and METADATA.ini)

Review

Reviewers will usually check the following:

Labels

  • Add the "work in progress" label if you do not want the PR to be reviewed yet.
  • Add the "ready to merge" label if the build server is happy and also you
    say that everything is ready to be merged.
@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

Unfortunately, since a I have a lot of pending changes for another issue, I could not create a separate PR just for this tutorial, so I had to do it from my different account, not from @alexsaber . I hope it is OK for you.

It is okay but you do not need different accounts for different PRs. You can simply create different branches and create a PR for each of the branches. See https://help.github.com/en/articles/creating-a-pull-request

@markus2330 markus2330 added the cm2019s label Jun 10, 2019

@alexsaber

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

It is okay but you do not need different accounts for different PRs.
I know this, of course, it tried it first, but it led me to nowhere after 2 hours of moving commits, changing branches, etc.

Why is the build failing this time?! I haven't changed any code in the PR..... I added the release note.

@markus2330
Copy link
Contributor

left a comment

Very nice tutorial.

@@ -0,0 +1,109 @@
# Introduction

Running all the tests like the build server does it requires to have multiple dependencies preinstalled. To overcome this problem, instead of trying to install all the necessary dependencies on your own, an appropriate Docker image should be used. This way you can easily and quickly run all the tests.

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 10, 2019

Contributor
Suggested change
Running all the tests like the build server does it requires to have multiple dependencies preinstalled. To overcome this problem, instead of trying to install all the necessary dependencies on your own, an appropriate Docker image should be used. This way you can easily and quickly run all the tests.
Running all the tests like the build server requires to have multiple dependencies installed. To overcome this problem, instead of trying to install all the necessary dependencies on your own, an appropriate Docker image can be used. This way you can easily and quickly run all the tests.

## Who is this guide for?

For anyone who wants to run all the tests, like it is done by the build server, once changes are pushed to the repository.

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 10, 2019

Contributor
Suggested change
For anyone who wants to run all the tests, like it is done by the build server, once changes are pushed to the repository.
For anyone who wants to run all the tests, like it is done by the build server.

### 1. Pick a Docker image and pull it

Pick one of the available Docker images of Elektra. If you do not know the difference, just pick this one --> "build-elektra-debian-stretch". Unfortunately, it will take some time to download it, since it is pretty big, but you can be sure you'll have all the needed dependencies.

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 10, 2019

Contributor

Maybe also suggest some alpine image as alternative for slow connections.

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 10, 2019

Contributor

But I thought it was not possible to run all tests with the alpine image. If it is possible, then I can suppest using it in the tutorial.

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 10, 2019

Contributor

Yes you are right but you can also mention this limitation in the tutorial.

docker pull <image_name>:<tag_name>
```

EXAMPLE:

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 10, 2019

Contributor
Suggested change
EXAMPLE:
Example:
<image_name>:<tag_name>
```

EXAMPLE:

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 10, 2019

Contributor
Suggested change
EXAMPLE:
Example:
### 3. Build


After starting the contrainer, you should be automatically inside it in the working directory we set --> /home/jenkins/workspace

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 10, 2019

Contributor
Suggested change
After starting the contrainer, you should be automatically inside it in the working directory we set --> /home/jenkins/workspace
After starting the container, you should be automatically inside it in the working directory `/home/jenkins/workspace`.

After starting the contrainer, you should be automatically inside it in the working directory we set --> /home/jenkins/workspace

Create folder for building project

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 10, 2019

Contributor

Sentence incomplete? Or should this be a header?

Finally run the tests
```sh
make run_all
```

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 10, 2019

Contributor

Also describe how to install Elektra and run the installed tests (kdb run_all). Please also link to doc/TESTING.md.

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 10, 2019

Contributor

What do you mean? Elektra is built within the container anyway. Why do we need to install it?

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 10, 2019

Contributor

To also install Elektra make install and run then the installed tests. The build server also does this and it is possible that the installed tests have different results (e.g. if test data is missing). See scripts/jenkins/Jenkinsfile

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 10, 2019

Contributor

Oh, I didn't know it. So basically before executing make run_all make install should be executed first?

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 10, 2019

Contributor

Yes, some tests only run if Elektra is installed.

And there is a second set of tests to be executed with:

kdb run_all

These tests also need an installed Elektra.

If your tutorial should cover how to execute all tests, you should mention this.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

Why is the build failing this time?! I haven't changed any code in the PR..... I added the release note.

Simply read what the build server tells you. (You might need to change between the pipeline steps, as not always the faulty one opens). The problem seems that you need to reformat markdown.

Will you describe the reformatting in a separate tutorial?


### 1. Pick a Docker image and pull it

Pick one of the available Docker images of Elektra. If you do not know the difference, just pick this one --> "build-elektra-debian-stretch". Unfortunately, it will take some time to download it, since it is pretty big, but you can be sure you'll have all the needed dependencies.

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 10, 2019

Contributor

But I thought it was not possible to run all tests with the alpine image. If it is possible, then I can suppest using it in the tutorial.

Finally run the tests
```sh
make run_all
```

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 10, 2019

Contributor

What do you mean? Elektra is built within the container anyway. Why do we need to install it?

@alexsaber

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

Now these tests in mac fail

The following tests FAILED:
	 86 - testmod_dbus (Failed)
	 87 - testmod_dbusrecv (Failed)
Errors while running CTest
FAILED: tests/CMakeFiles/run_all 
cd /private/var/folders/sy/2x5qvs0n4lg18fry9jz4y21m0000gn/T/cirrus-ci-build/build && /private/var/folders/sy/2x5qvs0n4lg18fry9jz4y21m0000gn/T/cirrus-ci-build/build/scripts/run_all RelWithDebInfo
ninja: build stopped: subcommand failed.

The Cirrurs CI say it failed due to following error:
test change notification with cascaded parent key

../src/plugins/dbus/testmod_dbus.c:393: error in test_cascadedChangeNotification: string "user/tests/foo/bar" is not equal to "system/tests/testmod_dbusrecv/added"
	compared: keyName (toAdd) and context->receivedKeyName
testmod_dbus Results: 35 Tests done — 1 error.
        Start  89: testmod_directoryvalue
 68/272 Test  #87: testmod_dbusrecv ..............................***Failed    0.05 sec

What does it have to do with my tiny change that does not even involve code? If I get that many errors after trying to push a text file, I wonder if it is even possible to finish resolving my other issue with a lot of code changes...

@sanssecours

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

What does it have to do with my tiny change that does not even involve code?

Those tests are known to fail quite regularly. I restarted the failing build job 🍎 Mac.

@alexsaber

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

What does it have to do with my tiny change that does not even involve code?

Those tests are known to fail quite regularly. I restarted the failing build job 🍎 Mac.

Oh, thank you very much! It is good to hear.

@alexsaber

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

@markus2330 I took into consideration all your comments and pushed some more changes. Before I did this, I saw all the build checks finally succeeded. Can you please merge this PR then?

@markus2330 markus2330 requested a review from sanssecours Jun 10, 2019

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

We need to wait for the build server and maybe other reviewers. I did not check if the code actually works yet.

@markus2330 markus2330 removed the request for review from sanssecours Jun 10, 2019

@alexsaber alexsaber referenced this pull request Jun 11, 2019

Merged

Reformat with docker tutorial #2773

0 of 13 tasks complete
@sanssecours
Copy link
Member

left a comment

I followed the steps described in the tutorial. My computer reported a few failing tests:

  • testshell_markdown_tutorial_cascading,
  • testshell_markdown_tutorial_validation,
  • testkdb_error, and
  • testkdb_ensure

, and minor problems with the install procedure. Those problems might be related to my host OS (macOS 10.14.5) though.

make install
```

The number 10 can be changed as follows: number of supported simultaneous threads by your CPU + 2. But don't worry, this can only affect the speed of the building, it cannot really break it.

This comment has been minimized.

Copy link
@sanssecours

sanssecours Jun 12, 2019

Member

I think it would make sense to move these instructions closer to the command make -j 10. Right below the command would be one good option in my opinion.

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 12, 2019

Contributor

Done it. Thank you!

Just run this command:

```sh
make install

This comment has been minimized.

Copy link
@sanssecours

sanssecours Jun 12, 2019

Member

This step fails on my machine with the error:

-- Install configuration: "RelWithDebInfo"
CMake Error at cmake_install.cmake:36 (file):
  file cannot create directory: /usr/local/share/doc/elektra.  Maybe need
  administrative privileges.


Makefile:72: recipe for target 'install' failed
make: *** [install] Error 1

. It might make sense to add the option -DINSTALL_SYSTEM_FILES=OFF to the cmake command above.

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 12, 2019

Contributor

Thank you for your comment! Now I am getting this error too... Unfortunately, adding -DINSTALL_SYSTEM_FILES=OFF did not help. Does anyone have a suggestion on how to solve it? @markus2330

This comment has been minimized.

Copy link
@sanssecours

sanssecours Jun 12, 2019

Member

I think setting CMAKE_INSTALL_PREFIX to a newly created folder inside the repository (e.g. "$PWD/install") should help.

This comment has been minimized.

Copy link
@oleksandr-shabelnyk

oleksandr-shabelnyk Jun 12, 2019

Author Contributor

I am sorry, but I don't understand what do you mean. Can you make an example?

This comment has been minimized.

Copy link
@sanssecours

sanssecours Jun 12, 2019

Member

Can you make an example?

Just replace the current cmake command with:

cmake .. \
 -DBINDINGS="ALL;-DEPRECATED;-haskell" \
 -DPLUGINS="ALL;-DEPRECATED" \
 -DTOOLS="ALL" \
 -DENABLE_DEBUG=ON \
 -DKDB_DB_HOME="$PWD" \
 -DKDB_DB_SYSTEM="$PWD/.config/kdb/system" \
 -DKDB_DB_SPEC="$PWD/.config/kdb/system" \
 -DCMAKE_INSTALL_PREFIX="$PWD/install" \
 -DINSTALL_SYSTEM_FILES=OFF

, and installing should work.

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 13, 2019

Contributor

I am using Ubuntu 18.04 in a virtual machine.

This comment has been minimized.

Copy link
@sanssecours

sanssecours Jun 13, 2019

Member

I am using Ubuntu 18.04 in a virtual machine.

Interesting. Would it not make more sense to just use Docker directly? Anyway, it would be great, if someone who uses Linux as main OS, could check if the tests also fail on their machine.

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 13, 2019

Contributor

I could not install Elektra on Windows, so I decided to work entirely in a Linux virtual machine.

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 13, 2019

Contributor

I could not install Elektra on Windows, so I decided to work entirely in a Linux virtual machine.

What did you try? Can you write an issue about that please?

Interesting. Would it not make more sense to just use Docker directly?

Yes, I also wonder why you do not use docker directly?

Anyway, it would be great, if someone who uses Linux as main OS, could check if the tests also fail on their machine.

I can reproduce the same problem (3 failed tests:testshell_markdown_tutorial_cascading testshell_markdown_tutorial_validation testkdb_ensure). Something is tidy inside the docker image.

Please report these three issues separately, they seems to be unrelated to this tutorial.

@sanssecours any idea why our build server does not catch this problem?

This comment has been minimized.

Copy link
@sanssecours

sanssecours Jun 13, 2019

Member

@sanssecours any idea why our build server does not catch this problem?

Nope.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 12, 2019

Thank you for the review!

@oleksandr-shabelnyk please fix the CMake command so that the tutorial actually works as described.

oleksandr-shabelnyk and others added some commits Jun 12, 2019

@markus2330
Copy link
Contributor

left a comment

Thank you!


```sh
mkdir build-docker && cd build-docker
mkdir build-docker && cd build-docker && mkdir install

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 13, 2019

Contributor

mkdir -p build-docker/install && cd build-docker is shorter and more fail-safe

But, in any way, I get:

mkdir: cannot create directory ‘build-docker’: Permission denied

because the folder is owned by uid/gid 1000, which is not the user in docker (id -u 47110)

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 13, 2019

Contributor

Ok, I am going to fix it.

Just run this command:

```sh
make install

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 13, 2019

Contributor

I could not install Elektra on Windows, so I decided to work entirely in a Linux virtual machine.

What did you try? Can you write an issue about that please?

Interesting. Would it not make more sense to just use Docker directly?

Yes, I also wonder why you do not use docker directly?

Anyway, it would be great, if someone who uses Linux as main OS, could check if the tests also fail on their machine.

I can reproduce the same problem (3 failed tests:testshell_markdown_tutorial_cascading testshell_markdown_tutorial_validation testkdb_ensure). Something is tidy inside the docker image.

Please report these three issues separately, they seems to be unrelated to this tutorial.

@sanssecours any idea why our build server does not catch this problem?


Finally run the tests. There are two sets of tests. Run the first one with this command:

```sh
make run_all
```

For the second set to run remember to execute `make install` from the previous step. Run the second set with this command:
For the second set to run, remember to execute `make install` from the previous step. Run the second set with this command:

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 13, 2019

Contributor

To make kdb run_all work more than setting the PATH is needed. (Also export LD_LIBRARY_PATH="$PWD/install/lib:$PATH").

But even then the testkdb_ensure fails.

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 13, 2019

Contributor

What did you try? Can you write an issue about that please?

It was 4 months ago. I really do not remember what I did back then.

Yes, I also wonder why you do not use docker directly?

My whole dev environment is inside this VM. On my host OS, Windows, it is hard to map volumes. And there sometimes problems with the rights.

Please report these three issues separately, they seems to be unrelated to this tutorial.

Ok, you mean

  1. Failing tests
  2. Windows installation problems
  3. What is the 3rd issue?

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 13, 2019

Contributor

To make kdb run_all work more than setting the PATH is needed. (Also export LD_LIBRARY_PATH="$PWD/install/lib:$PATH").

But even then the testkdb_ensure fails.

Thank you! I will add it.

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 13, 2019

Contributor

As 3rd issue, you probably meant failing tests with kdb run_all. I added it as well.

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 14, 2019

Contributor

I fixed everything you requested. Is it OK now? I am really running extremely out of time with all these changes...
The building seems to get stuck. Can you restart it?

@markus2330
Copy link
Contributor

left a comment

Still not fixed?

@@ -67,7 +66,7 @@ So from your root project folder run the following:
docker run -it --rm \
-v "$PWD:/home/jenkins/workspace" \
-w /home/jenkins/workspace \
<image_name>:<tag_name>
hub-public.libelektra.org/<image_name>:<tag_name>

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 14, 2019

Contributor

Here the user discussed in the issue #2781 is still missing.

This comment has been minimized.

Copy link
@alexsaber

alexsaber Jun 16, 2019

Contributor

The proposed solution introduces other problems. If running with the current user, not just 1 test fails, but around 20 of them.

This comment has been minimized.

Copy link
@markus2330

markus2330 Jun 16, 2019

Contributor

Seems like the problem can only be fixed if you build the docker container correctly. See scripts/docker/README.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.