PR#14: Initial commit for downstream tests using standard test interface - rpms/python3

rpms / python3

#14 Initial commit for downstream tests using standard test interface

Closed 6 years ago by pviktori. Opened 6 years ago by rasibley.

rpms/ rasibley/python3 python3-tests into master

Initial commit for downstream tests using standard test interface

Rachel Sibley • 6 years ago

73251c5

tests/sys/Makefile

file added

+63

		`@@ -0,0 +1,63 @@`
		`+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~`
		`+ #`
		`+ # Makefile of /CoreOS/python/sys`
		`+ # Description: Sanity test for the sys module`
		`+ # Author: Petr Splichal <psplicha@redhat.com>`
		`+ #`
		`+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~`
		`+ #`
		`+ # Copyright (c) 2011 Red Hat, Inc. All rights reserved.`
		`+ #`
		`+ # This copyrighted material is made available to anyone wishing`
		`+ # to use, modify, copy, or redistribute it subject to the terms`
		`+ # and conditions of the GNU General Public License version 2.`
		`+ #`
		`+ # This program is distributed in the hope that it will be`
		`+ # useful, but WITHOUT ANY WARRANTY; without even the implied`
		`+ # warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR`
		`+ # PURPOSE. See the GNU General Public License for more details.`
		`+ #`
		`+ # You should have received a copy of the GNU General Public`
		`+ # License along with this program; if not, write to the Free`
		`+ # Software Foundation, Inc., 51 Franklin Street, Fifth Floor,`
		`+ # Boston, MA 02110-1301, USA.`
		`+ #`
		`+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~`
		`+`
		`+ export TEST=/CoreOS/python/sys`
		`+ export TESTVERSION=1.0`
		`+`
		`+ BUILT_FILES=`
		`+`
		`+ FILES=$(METADATA) runtest.sh Makefile PURPOSE`
		`+`
		`+ .PHONY: all install download clean`
		`+`
		`+ run: $(FILES) build`
		`+ ./runtest.sh`
		`+`
		`+ build: $(BUILT_FILES)`
		`+ chmod a+x runtest.sh`
		`+`
		`+ clean:`
		`+ rm -f *~ $(BUILT_FILES)`
		`+`
		`+`
		`+ include /usr/share/rhts/lib/rhts-make.include`
		`+`
		`+ $(METADATA): Makefile`
		`+ @echo "Owner: Petr Splichal <psplicha@redhat.com>" > $(METADATA)`
		`+ @echo "Name: $(TEST)" >> $(METADATA)`
		`+ @echo "TestVersion: $(TESTVERSION)" >> $(METADATA)`
		`+ @echo "Path: $(TEST_DIR)" >> $(METADATA)`
		`+ @echo "Description: Sanity test for the sys module" >> $(METADATA)`
		`+ @echo "Type: Sanity" >> $(METADATA)`
		`+ @echo "TestTime: 5m" >> $(METADATA)`
		`+ @echo "RunFor: python" >> $(METADATA)`
		`+ @echo "Requires: python" >> $(METADATA)`
		`+ @echo "Priority: Normal" >> $(METADATA)`
		`+ @echo "License: GPLv2" >> $(METADATA)`
		`+ @echo "Confidential: no" >> $(METADATA)`
		`+ @echo "Destructive: no" >> $(METADATA)`
		`+`
		`+ rhts-lint $(METADATA)`

tests/sys/PURPOSE

file added

		`@@ -0,0 +1,8 @@`
		`+ PURPOSE of /CoreOS/python/sys`
		`+ Description: Sanity test for the sys module`
		`+ Author: Petr Splichal <psplicha@redhat.com>`
		`+`
		`+ Basic sanity test for the sys module. For now included just coverage for`
		`+ (sys.platform == "linux2") also on machines running Linux 3 kernels`
		`+ for python2.7 and python3.2. Starting with python3.3, sys.platform will be just`
		`+ "linux". See http://bugs.python.org/issue12326`

tests/sys/runtest.sh

file added

+63

		`@@ -0,0 +1,63 @@`
		`+ #!/bin/bash`
		`+ # vim: dict=/usr/share/beakerlib/dictionary.vim cpt=.,w,b,u,t,i,k`
		`+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~`
		`+ #`
		`+ # runtest.sh of /CoreOS/python/sys`
		`+ # Description: Sanity test for the sys module`
		`+ # Author: Petr Splichal <psplicha@redhat.com>`
		`+ #`
		`+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~`
		`+ #`
		`+ # Copyright (c) 2011 Red Hat, Inc. All rights reserved.`
		`+ #`
		`+ # This copyrighted material is made available to anyone wishing`
		`+ # to use, modify, copy, or redistribute it subject to the terms`
		`+ # and conditions of the GNU General Public License version 2.`
		`+ #`
		`+ # This program is distributed in the hope that it will be`
		`+ # useful, but WITHOUT ANY WARRANTY; without even the implied`
		`+ # warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR`
		`+ # PURPOSE. See the GNU General Public License for more details.`
		`+ #`
		`+ # You should have received a copy of the GNU General Public`
		`+ # License along with this program; if not, write to the Free`
		`+ # Software Foundation, Inc., 51 Franklin Street, Fifth Floor,`
		`+ # Boston, MA 02110-1301, USA.`
		`+ #`
		`+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~`
		`+`
		`+ # Include Beaker environment`
		`+ . /usr/share/beakerlib/beakerlib.sh \|\| exit 1`
		`+`
		`+ # Packages to be tested`
		`+ PACKAGES="${PACKAGES:-python3}"`
		`+ # Other required packages`
		`+ REQUIRES=${REQUIRES:-}`
		`+ # Binary name parametrized`
		`+ PYTHON="${PYTHON:-python3}"`
		`+`
		`+ # print("linux2" if sys.version_info < (3, 3) else "linux") for python2.4 on RHEL5`
		`+ PLATFORM_CMD='import sys; print(sys.version_info < (3, 3) and "linux2" or "linux")'`
		`+`
		`+ rlJournalStart`
		`+ rlPhaseStartSetup`
		`+ rlAssertRpm --all`
		`+ rlAssertBinaryOrigin $PYTHON`
		`+ rlRun "TMP=\$(mktemp -d)" 0 "Creating tmp directory"`
		`+ rlRun "pushd $TMP"`
		`+ rlPhaseEnd`
		`+`
		`+ rlPhaseStartTest`
		`+ rlRun -l "$PYTHON -V" 0 "Query version of the binary"`
		`+ PLATFORM_STR=$($PYTHON -c "$PLATFORM_CMD")`
		`+ rlRun "$PYTHON -c 'import sys; print(sys.platform)' \| tee output" \`
		`+ 0 "Inspecting the value of sys.platform"`
		`+ rlAssertGrep $PLATFORM_STR "output"`
		`+ rlPhaseEnd`
		`+`
		`+ rlPhaseStartCleanup`
		`+ rlRun "popd"`
		`+ rlRun "rm -r $TMP" 0 "Removing tmp directory"`
		`+ rlPhaseEnd`
		`+ rlJournalPrintText`
		`+ rlJournalEnd`

tests/tests.yml

file added

+14

		`@@ -0,0 +1,14 @@`
		`+ ---`
		`+ # This first play always runs on the local staging system`
		`+ - hosts: localhost`
		`+ roles:`
		`+ - role: standard-test-beakerlib`
		`+ tags:`
		`+ - classic`
		`+ - atomic`
		`+ - container`
		`+ tests:`
		`+ - sys`
		`+ required_packages:`
		`+ - python`
		`+ - which`

rasibley commented 6 years ago

Adds tests according to the CI wiki [0] specifically the standard test interface in the spec [1].

The playbook includes Tier1 level test cases that have been tested in the following contexts and is passing reliably: Atomic Host, Docker, and Classic.
Test logs are stored in the Artifacts directory.

The following steps are used to execute the tests using the standard test interface:

Atomic
sudo ANSIBLE_INVENTORY=$(test -e inventory && echo inventory || echo /usr/share/ansible/inventory) TEST_SUBJECTS=../atomic.qcow2 TEST_ARTIFACTS=$PWD/artifacts ansible-playbook --tags atomic tests.yml
Docker
sudo ANSIBLE_INVENTORY=$(test -e inventory && echo inventory || echo /usr/share/ansible/inventory) TEST_DEBUG=1 TEST_SUBJECTS=docker:docker.io/library/fedora:26 TEST_ARTIFACTS=$PWD/artifacts ansible-playbook --tags container tests.yml
Classic
sudo ANSIBLE_INVENTORY=$(test -e inventory && echo inventory || echo /usr/share/ansible/inventory) TEST_SUBJECTS="" TEST_ARTIFACTS=$PWD/artifacts ansible-playbook --tags classic tests.yml

Test Logs: (If you would like a pointer to the complete log, I can include that as well)

Atomic
04:07:44 TASK [standard-test-beakerlib : Execute beakerlib tests]
04:07:50 changed: [../atomic.qcow2] => (item=sys)
04:07:50
04:07:50 TASK [standard-test-beakerlib : Make the master test summary log artifact]
04:07:50 changed: [../atomic.qcow2] => (item=sys)
04:07:50
04:07:50 TASK [standard-test-beakerlib : Pull out the logs]
04:07:51 changed: [../atomic.qcow2]
04:07:51
04:07:51 TASK [standard-test-beakerlib : Check the results]
04:07:51 changed: [../atomic.qcow2]
04:07:51
04:07:51 PLAY RECAP
04:07:51 ../atomic.qcow2 : ok=13 changed=9 unreachable=0 failed=0
04:07:51
04:07:51 ###################################
04:07:51 Test results:
04:07:51 ###################################
04:07:51 PASS sys
Docker
04:10:53 TASK [standard-test-beakerlib : Execute beakerlib tests]
04:10:58 changed: [656b8fda91e7a304dfc37119c3af722a59cafb8b83548e2d2a932421a4903844] => (item=sys)
04:10:58
04:10:58 TASK [standard-test-beakerlib : Make the master test summary log artifact]
04:10:58 changed: [656b8fda91e7a304dfc37119c3af722a59cafb8b83548e2d2a932421a4903844] => (item=sys)
04:10:58
04:10:58 TASK [standard-test-beakerlib : Pull out the logs]
04:10:59 changed: [656b8fda91e7a304dfc37119c3af722a59cafb8b83548e2d2a932421a4903844]
04:10:59
04:10:59 TASK [standard-test-beakerlib : Check the results]
04:10:59 changed: [656b8fda91e7a304dfc37119c3af722a59cafb8b83548e2d2a932421a4903844]
04:11:00
04:11:00 PLAY RECAP
04:11:00 656b8fda91e7a304dfc37119c3af722a59cafb8b83548e2d2a932421a4903844 : ok=15 changed=11 unreachable=0 failed=0
04:11:00
04:11:00 ###################################
04:11:00 Test results:
04:11:00 ###################################
04:11:00 PASS sys
Classic
04:05:04 ==> default: TASK [standard-test-beakerlib : Execute beakerlib tests]
04:05:07 ==> default: changed: [localhost] => (item=sys)
04:05:07 ==> default:
04:05:07 ==> default: TASK [standard-test-beakerlib : Make the master test summary log artifact]
04:05:08 ==> default: changed: [localhost] => (item=sys)
04:05:08 ==> default:
04:05:08 ==> default: TASK [standard-test-beakerlib : Pull out the logs]
04:05:08 ==> default: changed: [localhost]
04:05:08 ==> default:
04:05:08 ==> default: TASK [standard-test-beakerlib : Check the results]
04:05:08 ==> default: changed: [localhost]
04:05:08 ==> default:
04:05:08 ==> default: PLAY RECAP
04:05:08 ==> default: localhost : ok=15 changed=11 unreachable=0 failed=0
04:05:08 ==> default: ++ '[' 0 -ne 0 ']'
04:05:08 ==> default: ++ cat /root/python3/artifacts/test.log
04:05:08 ==> default: PASS sys
04:05:08 ==> default: ++ grep -ve '^PASS' /root/python3/artifacts/test.log
04:05:08 ==> default: PASS: all tests passed.
04:05:08 ==> default: ++ '[' 1 -eq 1 ']'

Tests will be enabled in CI, yet gating is currently disabled, so nothing will change. However eventually gating will be enabled. Tests will run on each dist-git commit, they are not triggered on koji builds and if you are using FMN, it should notify you of failures normally.

The RH QE maintainer contact in case you have questions: optak@redhat.com
The idea is that these tests become yours just as you're maintaining the package, there will of course be people around if you have questions or troubles.

[0] https://fedoraproject.org/wiki/CI
[1] https://fedoraproject.org/wiki/Changes/InvokingTests

churchyard commented 6 years ago

I'm a bit confused here, this is 152 lines of boilerplate to test that sys.platform is "linux"?

torsava commented 6 years ago

Are we supposed to extend the tests ourselves? Perhaps run the Python test suite inside?

rasibley commented 6 years ago

Hello, I apologize for the late response, I was on PTO and attending a conference the last few days. I also wanted to check in with the downstream maintainer to provide more details. The main test (runtest.sh) is about 30+ lines of code and is simply a sanity check for python and sys module. We hope to add more smoke tests which will provide more extensive testing later on. The tests.yml file is required to invoke the test by the standard test interface, and the Makefile and Purpose are just supporting files.

You are welcome to extend the test, or we can look into extending, in addition to looking for additional tests downstream which we can add later.

The tests will be enabled and executed in CI, using the using the standard test interface as described in the following wiki pages:
https://fedoraproject.org/wiki/CI
https://fedoraproject.org/wiki/Changes/InvokingTests

I hope that helps, please let us know if you have more questions.

pviktori commented 6 years ago

I am lost. I don't think any Python maintainer in Fedora is familiar enough with Ansible and beakerlib to maintain these tests, yet you say "the idea is that these tests become yours just as you're maintaining the package". The wiki pages linked above say nothing about how to write beakerlib tests, except how to wrap them "if (we) have a set of beakerlib tests". We do not have a set of beakerlib tests. It seems it would be much easier to write and wrap a script test.

In particular, I am thoroughly baffled by the Makefile:

What's $(METADATA). Is the specified owner, Petr Splichal <psplicha@redhat.com>, responsible for maintaining these tests? (Above you say optak@redhat.com is the RH QE maintainer contact).
What's /CoreOS/python/sys?
Where's /usr/share/rhts/lib/rhts-make.include or rhts-lint? I couldn't find them in the Fedora repos.
What uses TestTime?
etc., etc.

Also, the licencing is unclear. Which files under the GPL license, and what's under Fedora's default specfile license (MIT)? (Only some files have a GPL header, but METADATA just says License: GPLv2.) How does/will GPLv2 limit including stuff under other licences?

dperpeet commented 6 years ago

I am lost. I don't think any Python maintainer in Fedora is familiar enough with Ansible and beakerlib to maintain these tests, yet you say "the idea is that these tests become yours just as you're maintaining the package". The wiki pages linked above say nothing about how to write beakerlib tests, except how to wrap them "if (we) have a set of beakerlib tests". We do not have a set of beakerlib tests. It seems it would be much easier to write and wrap a script test.

Indeed, script tests can be more straightforward at times. If we clean up these files and add more comments, would that help with making them maintainable? The specific feedback items are already very helpful to understand which parts you worry about, and I'm very grateful for your detailed review.

The beakerlib name stems from its origins, since it doesn't actually depend on beaker.

Also, the licencing is unclear. Which files under the GPL license, and what's under Fedora's default specfile license (MIT)? (Only some files have a GPL header, but METADATA just says License: GPLv2.) How does/will GPLv2 limit including stuff under other licences?

If the files have a license at the top, that is the license for the file. If no license is stated, I believe they fall under the https://fedoraproject.org/wiki/Legal:Fedora_Project_Contributor_Agreement and therefore are MIT, since this is code and not content (per the definitions of that agreement). Do you agree @rasibley ?

GPL is marked as a "Good License" accoring to the Licensing docs: https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses

pviktori commented 6 years ago

Indeed, script tests can be more straightforward at times. If we clean up these files and add more comments, would that help with making them maintainable?

Making the tests more straightforward would be great. Adding helpful comments too, but self-explanatory code is always better :)

Another piece of information that would help is your vision of what these tests should ultimately do. While this pull request works as an example of how the tests can be implemented, it doesn't tell us why implementing them will help Python/Fedora.
Python's self-test suite is already executed in the RPM %check section. Is the plan to replace %check? Or should the CI tests ultimately include tests from outside the CPython upstream project?

The specific feedback items are already very helpful to understand which parts you worry about, and I'm very grateful for your detailed review.

Thank you for responding!

If the files have a license at the top, that is the license for the file. If no license is stated, I believe they fall under the https://fedoraproject.org/wiki/Legal:Fedora_Project_Contributor_Agreement and therefore are MIT, since this is code and not content (per the definitions of that agreement). Do you agree @rasibley ?
GPL is marked as a "Good License" accoring to the Licensing docs: https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses

OK. Then either the metadata needs to say something like "License: GPLv2 and MIT", or there needs to be a clarification of what this line means. (Or better, if it can, that line should be removed entirely.)

GPL is marked as a "Good License" accoring to the Licensing docs: https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses

It is, but it is incompatible with many other good licences (notably GPLv3), so it might limit our options in the future. It would be easier for us if the GPLv2 code can be eliminated.

pviktori commented 6 years ago

I'll close this pull request; lzachar promised to send a more appropriate one.