/snapcraft

Add snapcraft examples to scaffold getting started examples #513

Merged
merged 39 commits into from Jun 3, 2016
+607 −221

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@didrocks @kyrofa @elopio @sergiusens @markshuttle
@didrocks
Contributor

didrocks commented May 24, 2016

Adds unit and integration tests for it, reusing and making more generic all demos tests helper.

Note that first commit is on another PR (move examples to demos): ubuntu-core#506 which needs to be merged first for review clarity.

This one will then be followed up by one PR changing the url for eamples 05- to point to this repository, and as well for disabling the manual run_demos() additional run when the "example" target is selected once CI changes the runner script.
@didrocks
Contributor

didrocks commented May 24, 2016

Rebased on latest master changes
@didrocks
Contributor

didrocks commented May 24, 2016

mock doesn't behave the same between python 3.4 (travis CI) and 3.5 (xenial), seems we have to patch builtins to be compatible in 3.4, gave it a shot for Travis CI.
@didrocks
Contributor

didrocks commented May 25, 2016

ok, pushed and hoping this is the last time I had to rebase :)

So, for now, we are now shipping (as per demand) the binary in the snapcraft package (it needs to be installed alongside snapcraft). Ship them the scaffolding get started with it.

I still have hope to get it renamed from snapcraft-examples to "snapcraft learn", but that will be in another separated PR.

Do you mind merging this please? I'm away for a couple of days, FYI
@kyrofa
Member

kyrofa commented May 27, 2016 •
Edited 1 time

Note Mark's comment on bug #1586137: "Let's call this snapcraft examples please, not snapcraft-examples."

@didrocks didrocks changed the title from Add snapcraft-examples binary to scaffold getting started examples to Add snapcraft examples to scaffold getting started examples May 30, 2016

demos_tests/__main__.py
@@ -67,7 +68,7 @@ def main():
# them again.
argv = [sys.argv[0]]
argv.append('discover')
- argv.append('demos_tests')
+ argv.append(os.path.dirname(__file__).split(os.path.sep)[-1])
@elopio

elopio Jun 1, 2016

Member

I think you want here os.path.basename, instead of split and [-1]

@didrocks

didrocks Jun 1, 2016

Contributor

Oh, good catch!, I must have my head broken when writing this :)

examples/01-hello-world-cli/snapcraft.yaml
+version: 0.1
+summary: Hello World simple C app
+description: |
+ This demo is intended to show how to build a C app
@elopio

elopio Jun 1, 2016

Member

s/demo/example ?

@didrocks

didrocks Jun 1, 2016

Contributor

Well, you can ignore the current examples directory, they are being renamed as "tour" and Mark is rewriting them.

snapcraft/main.py
+ dest_dir = os.path.join(dest_dir, _SNAPCRAFT_TOUR_DIR)
+ shutil.copytree(get_examplesdir(), dest_dir)
+
+ print("Snapcraft tour initialized in {}.\n"
@elopio

elopio Jun 1, 2016

Member

I think this should be logger.info, but we have some inconsistencies here in how to handle errors and messages in main. That's a bug for the next milestone.

@didrocks

didrocks Jun 1, 2016

Contributor

ok, I can rename and use logger.info if the output is the same.

@elopio
Member

elopio commented Jun 1, 2016

This is great didrocks. For the tests, what if instead of having a demos_tests and examples_tests, we put them all in a user_tests dir or snaps_tests dir?
For the point of view of our CI pipeline, the two serve the same purpose, so I see no reason why we would like to have them separated. I would prefer a single ./runtests command to run both.
@didrocks
Contributor

didrocks commented Jun 1, 2016

I'm all for having just one single _tests directory. Let me try to play with this (have both under snaps_tests/ dir) and then, looping on the 2 subdirs to tests tour/ (replacing examples/) and demos/.

Do you still want the CI target to be "examples" or should be replace it with "snaps"?

I'll rebase on this, push Mark's current changes (so /!\ not to merge, and Mark did some changes in term of help text content in the same branch, so the diff will be a little bit larger…), rebasing on master and implementing the above change for pre-review.
@elopio
Member

elopio commented Jun 1, 2016

I'd like the ci target to have the same name of that new demos+tours tests directory.
We will have to change the jobs in jenkins, which is easy but requires syncronization or dealing with failed executions. So I'll just keep an eye on your proposals. Let me know if you need a hand dealing with the test suites.
@didrocks
Contributor

didrocks commented Jun 1, 2016

Excellent! I'll push something with both targets "examples" and "snaps" first, running the some run_snaps tests (to ensure we always run something and I can see I didn't make a booboo ;)). We can even land it, and then, once CI has migrated (when most of current PR are rebased on master to change the targets), do another PR to remove the "examples" target.

I'll go for pushing the update today and will ping you :)
@sergiusens
Collaborator

sergiusens commented Jun 2, 2016

If we land this tomorrow, can we update .gitignore?

The move from examples to demos is driving my status nuts :-P
@didrocks
Contributor

didrocks commented Jun 2, 2016 •
Edited 1 time

Here we go, rebased on master and with changes to have demos_tests and tour_tests under the same directory.
Note that the command is backward-comptible:
./runtests examples and ./runtests snaps both runs the same tests.

Seems that autopkgtest are down again.

Examples tests run both the old demos_tests and snaps_tests and the result is: [1;32mEverything passed�[0m. Unsure why the result doesn't reflect this.

(Note that the docstring changes are comming directly from Mark's branch as this one includes his current modifications: https://github.com/markshuttle/snapcraft/commits/tour)
bin/snapcraft-examples
+ sys.path = [topdir] + sys.path
+
+
+if __name__ == '__main__':
@sergiusens

sergiusens Jun 2, 2016

Collaborator

I thought this was supposed to be snapcraft tour (no - dash)

What is this for, I might be missing something?

@didrocks

didrocks Jun 2, 2016

Contributor

It's me, this binary should be removed and is useless, I forgot to delete it. Pushing the removal right away

setup.py
@@ -43,7 +43,7 @@
'snapcraft.plugins',
'snapcraft.storeapi'],
package_data={'snapcraft.internal': ['manifest.txt']},
- scripts=['bin/snapcraft'],
+ scripts=['bin/snapcraft', 'bin/snapcraft-examples'],
@sergiusens

sergiusens Jun 2, 2016

Collaborator

ditto, what is snapcraft-examples?

@didrocks

didrocks Jun 2, 2016

Contributor

yeah, leftover as well (now fixed and pushed). Good catch!

snaps_tests/demos_tests/test_ros.py
import os
import subprocess
-class ROSTestCase(demos_tests.ExampleTestCase):
+class ROSTestCase(snaps_tests.SnapsTestCase):
demo_dir = 'ros'
@elopio

elopio Jun 2, 2016

Member

s/demo_dir/snap_content_dir

@didrocks

didrocks Jun 2, 2016

Contributor

Good catch! Fixed

@sergiusens
Collaborator

sergiusens commented Jun 2, 2016

demostests FAIL non-zero exit status 1
@sergiusens
Collaborator

sergiusens commented Jun 2, 2016

/usr/bin/python3: No module named demos_tests

didrocks and others added some commits May 24, 2016

@didrocks @elopio
snapcraft-examples binary to scaffold getting started examples 
Add helpers and unit tests around those examples.
e0b810f
@didrocks @elopio
Add the examples themselves in examples/ directory 
Those examples are the getting started one. Note that for example 05-, we will
then reference the snapcraft git tree itself once this is merged.
a484784
@didrocks @elopio
Rename keyword and make demo_tests helper more generic 
We want to reuse them to add integration tests to examples as well as demos
using the same code (symlinks).
69b8cbe
@didrocks @elopio
Add integration tests for examples 
* Reuse (symlinks) helpers from demos_tests
* Add tests for each example
13c002b
@didrocks @elopio
Add a temporary demo run as part of examples 
This is to ensure we are still building and testing demos until CI adds
the demo target testrun.
45c4ffd
@didrocks @elopio
Install additional binary and examples in main package
f9efd16
@didrocks @elopio
Move to "snapcraft examples" 
Remove tweaked snapcraft-examples command to an independent snapcraft examples
argument.
5c2024f
@didrocks @elopio
Remove previous examples
becbf83
@didrocks @elopio
Add GNU Hello
48a692a
@didrocks @elopio
Add snapcraft.yaml
8e6e61c
@markshuttle @elopio
Start the tour!
a61a71b
@didrocks @elopio
Remove examples/ to tour/
9da196d
@didrocks @elopio
Remove old tests for older examples while renaming to tour
702b573
@didrocks @elopio
Adapt snapcraft example to snapcraft tour 
Copy from the new directory
Change CI to trigger new tests
Remove old tests for previous tour examples and replace with the new "GNU hello" one.
Adapt unit and integration tests for this.
8d10090
@didrocks @elopio
Add standard snap gitignore
f1195ab
@markshuttle @elopio
Add initial hello snap
83d376c
@markshuttle @elopio
Tweak initial snapcraft.yaml
e827af5
@didrocks @elopio
Remove imported version of 01-hello
27628f2
@didrocks @elopio
Support running tests in subdirectories 
Structure of the test runner should follow the tour structure. Then, we run
and build from the correct subdirectory the tests.
723ade3
@markshuttle @elopio
Refining tour snapcraft.yaml
2f2cb13
@markshuttle @elopio
Clean up common source keywords help text
3a4c2f6
@markshuttle @elopio
Clean up docstrings used for help text
d547eaf
@didrocks @elopio
No need for split os.path.sep
57dfd35
@didrocks @elopio
Move test content to snaps_tests 
The idea is to be merge demos_tests and tour_tests in a single test structure.
This one has 2 subdirectories: demos_tests and tour_tests.
496c12c
@didrocks @elopio
Add a way to run tests in subdirectories 
It's better to separate tests runs, so let's run them in subdirectories
for each kind of tests (demos and tour). Add a regexp to know where to look
for the snaps, and then, follow natural relative path
f7b0892
@didrocks @elopio
Point the runner to snaps_tests now and rename modules 
All modules now refers to the same snaps_tests one. Also, tweak the test runner
to include this directory in static checks and run build tests on both
"examples" (for backward compatibility) and "snaps" targets.
fca8e43
@didrocks @elopio
Add some .gitignore 
The ones in the tour are inline, so that they are copied to the final
scaffolded directory for our users to follow best practices.
4c0f249
@didrocks @elopio
Add another tests for second example in tour
e2a9f52
@didrocks @elopio
Fix unit test expected message for new output
2fd63c6
@didrocks @elopio
Disable "tour" command for now 
Only disable user-visible feature for now. This will enable us to easily
get the additional tour examples in the next release.
4a7b3c1
@didrocks @elopio
Remove useless binary now
53bb317
@didrocks @elopio
Rename demo_dir in test_ros as well
e8f9cde
@didrocks @elopio
Remove snapcraft-example from install as well
52f7b31
@didrocks @elopio
Fix missing comma in hello snaps output comparison
580f31b
@elopio
Updated the name of the debian tests.
c477625
@elopio
skip the failing test.

Some checks were not successful

2 failing and 2 successful checks
@snappy-m-o
autopkgtest
Details
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
@snappy-m-o
Examples tests — Success
Details
coverage/coveralls — Coverage decreased (-0.05%) to 95.677%
Details
f51573d
@elopio
Fixed static errors.

Some checks were not successful

1 failing and 3 successful checks
@snappy-m-o
autopkgtest
Details
@snappy-m-o
Examples tests — Success
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.0008%) to 95.728%
Details
609ee78
@elopio
One more skip.

All checks have passed

4 successful checks
@snappy-m-o
Examples tests — Success
Details
@snappy-m-o
autopkgtest — Success
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.0008%) to 95.728%
Details
fd609af
@elopio
Member

elopio commented Jun 3, 2016

There is one error in one of the tests, but they are failing anyway. Skipped in #544
@didrocks
Contributor

didrocks commented Jun 3, 2016

Seems like the forked version is now passing tests (I have no idea about the bad system calls though, seems like an apparmor issue when trying echo?)
@didrocks
Use now bash --version instead of -c for tests 
bash -c is using an inappropriate syscalls breaking confinement. This will
get worked out with upstream.
Use --version instead and just ensure the command works.

Replace thus the skip tests message with the other one (due to IS)

All checks have passed

4 successful checks
@snappy-m-o
Examples tests — Success
Details
@snappy-m-o
autopkgtest — Success
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage increased (+0.0008%) to 95.728%
Details
1ca8642
@didrocks
Contributor

didrocks commented Jun 3, 2016

retest this please

(seems the autopkgtest issue was a transient failure, connection dropped)
@didrocks
Contributor

didrocks commented Jun 3, 2016

and everything pass! This branch should be good to merge, containing latest elopio's changes :)

@sergiusens sergiusens merged commit 7f334a5 into snapcore:master Jun 3, 2016
4 checks passed

4 checks passed

Examples tests Success
Details
autopkgtest Success
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.0008%) to 95.728%
Details

kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017

@didrocks @sergiusens
Add snapcraft examples to scaffold getting started tour (#513) 
Add helpers and unit tests around those tours

LP: #1586137
f12e2a1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment