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

CI: generic runtime tests #13785

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

aparcar
Copy link
Member

@aparcar aparcar commented Oct 27, 2020

Requested via #13589

@aparcar aparcar force-pushed the jefferyto-runtime branch 2 times, most recently from 4fd3b28 to df3a23f Compare October 27, 2020 18:54
Most compiled programs offer a way to output the installed version,
usually via arguments like `-v`, or `--version`. This fact can be used
to runtime test compiled in a generic way. If the binary prints the
compiled version it proves that the program loaded correctly on the
specific architecture.

While this obviously lacks testing of actual functionallity, it proves a
package successfully compiled, installed and loaded related binaries.

In cases where no package specific `test.sh` is found, try a variation
of common way to print a programs version.

Signed-off-by: Paul Spooren <mail@aparcar.org>
@aparcar
Copy link
Member Author

aparcar commented Oct 27, 2020

@jefferyto Here we face the first problem. The vim package contains a tool called xxd which is an entirely different version:

Testing package xxd in version 8.2 from vim
Installing xxd (8.2-2) to root...
Configuring xxd.
Use generic test
Test executable /usr/bin/xxd

It seems to be something like this:

$ xxd -v
xxd V1.10 27oct98 by Juergen Weigert

Signed-off-by: Paul Spooren <mail@aparcar.org>
@jefferyto
Copy link
Member

I don't want to comment too much on the immediate issue with xxd because I'm not crazy about this PR as it stands. I think doing everything inside the container is too inflexible and you have no access to the package Makefile.

I suggest something like this (it doesn't need to be all done in one PR), for each package/subpackage:

  • Checking the package Makefile, if the package has opted into tests (as mentioned in Runtime test ideas/enhancements #13589 (comment))
    • If the package does not have a custom test script, output the default test script to something like custom_test.sh or custom_test_<subpackage-name>.sh
    • (optional) If the package has a customized command line argument (instead of --version -version ...), output the default test script with the custom command line argument into custom_test.sh / custom_test_<subpackage-name>.sh (perhaps there are other variables in the default test script that can be customized in a similar manner)
    • If the package has a custom test script, output it into custom_test.sh / custom_test_<subpackage-name>.sh
  • Inside the container
    • If there is a custom_test.sh / custom_test_<subpackage-name>.sh, run it
    • Otherwise if there is a test.sh, run that

This way the content of test.sh can even be completely defined inside the Makefile (similar to config options, they can be defined inside the Makefile or sourced from a separate file).

@champtar
Copy link
Member

We expect contributor to be able to fix CI failure, so we need to be able to replicate what the CI does as easily as possible
=> make package/aaa/test would be really nice
And also CI should be green for working packages, it's nice to test more things but if we end up ignoring 90% of the times is it really better

@aparcar
Copy link
Member Author

aparcar commented Nov 18, 2020

Sorry this slipped through my notifications.

everything inside the container is too inflexible

Please elaborate what parts are too inflexible.

you have no access to the package Makefile

The test.sh script is placed literally next to the Makefile and therefore accessible from the container. In fact the entire packages.git is mounted within the Docker container.

We could use Makefile define to define tests rather than having separate files, not sure if that's a big advantage.

=> make package/aaa/test would be really nice

How do you envision this to be implemented? Your host system is unlikely to support all these architectures, likely not even x86-musl. Then you need a working opkg installing dependencies. You can make it work, but that's plenty of work. One could automatically spin up a dozen Docker containers, but then again that's what the CI is for. My lazy approach would be that people just draft PRs and push until the CI is happy. I don't expect anyone to test their PRs for 10 archs.

echo "Generic test failed"
exit 1
else
echo "Test succesful or"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be without or?

Suggested change
echo "Test succesful or"
echo "Test successful"

echo "Test succesful or"
fi
else
echo "No executeable found in package $PKG_NAME"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
echo "No executeable found in package $PKG_NAME"
echo "No executable found in package $PKG_NAME"

@BKPepe
Copy link
Member

BKPepe commented Dec 29, 2020

Tested with lighttpd. Seems OK to me! More tested packages will follow soon.

@aparcar
Copy link
Member Author

aparcar commented Dec 29, 2020

I'd like to get more input from @champtar and @jefferyto before continuing.

@BKPepe
Copy link
Member

BKPepe commented Jan 26, 2021

  • (optional) If the package has a customized command line argument (instead of --version -version ...), output the default test script with the custom command line argument into custom_test.sh / custom_test_<subpackage-name>.sh (perhaps there are other variables in the default test script that can be customized in a similar manner)

I agree on this one. We should have something like this.
There might be some packages, which does not have e.g. --version, -version and it that case check fails and in the logs, there is: generic test failed. In that case, there could be fallback to custom test or make it even complicated:

  1. try to check for version
  2. try to show help
  3. try to run app without any parameters to check if it runs and that it does not segfault or does not show any errors
  4. have custom script if method 1.,2. fails (they are not there).

Because I doubt that anyone wants to create his/her test.sh or ship the generic test.sh with each package folder, so having something by default 👍

TL;DR: I agree with @jefferyto's suggestions, but one difference that all packages will be using tests even though could be complicated for libraries.

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

Successfully merging this pull request may close these issues.

4 participants