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

Travis CI + Flake8 #17

Closed
davelab6 opened this issue May 16, 2016 · 30 comments
Closed

Travis CI + Flake8 #17

davelab6 opened this issue May 16, 2016 · 30 comments
Assignees
Labels
Milestone

Comments

@davelab6
Copy link
Contributor

@davelab6 davelab6 commented May 16, 2016

  • Set up Travis CI with flake8

http://victorlin.me/posts/2013/09/19/fall-in-love-with-continuous-testing-and-integration-travis-ci

Eg https://github.com/edx/harprofiler/pull/52/files

https://pypi.python.org/pypi/tox might be good too

  • Set 'master' branch to be a 'protected branch' in Github to prevent regressions
  • Fix all errors reported by flake8
@samdroid-apps
Copy link

@samdroid-apps samdroid-apps commented May 16, 2016

If you find how to do this, please post it here. I would love a simple
Flake8 CI runner for all sugarlabs repos!!!

On Mon, May 16, 2016 at 11:17 PM, Dave Crossland
notifications@github.com wrote:

Set up Travis CI with flake8


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented May 17, 2016

I got stuck with the root requirement :( https://travis-ci.org/davelab6/edit-fonts-activity

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented May 26, 2016

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented May 26, 2016

https://pygobject.readthedocs.io/en/latest/memory_leaks.html suggests we could automate checking for memory leaks

@YashAgarwal
Copy link
Contributor

@YashAgarwal YashAgarwal commented Jun 8, 2016

Travis CI shows a build error after the ./osbuild shell command because there is no response after 100ms

$ ./osbuild shell
$ sudo broot run osbuild shell
= Available commands =
docs
dist
check
run
build
karma
clean
pull
See also http://developer.sugarlabs.org/dev-environment.md.html
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
The build has been terminated

This is the .travis.yml file used : https://github.com/sugarlabs/edit-fonts-activity/blob/yash-v0/.travis.yml

@YashAgarwal
Copy link
Contributor

@YashAgarwal YashAgarwal commented Jun 8, 2016

here is the complete error log for the above mentioned build https://raw.githubusercontent.com/sugarlabs/edit-fonts-activity/yash-v0/log.txt

@samdroid-apps
Copy link

@samdroid-apps samdroid-apps commented Jun 8, 2016

Osbuild shell just runs a shell. Shells wait for user input to do commands. You might need to supply some user input. What do you want to do?

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jun 14, 2016

@YashAgarwal Alright! You've got the same error (no response) because you're opening a shell and then not giving it valid commands as input (and initially, not giving it any commands at all :) And now @samdroid-apps has just explained how to send input to the shell - thank you Sam! :)

For the travis file, it doesn't really matter which section of the travis file you execute the commands in; the different sections are almost merely 'syntactic sugar' - if you wanted to put all the commands in the install section that would mean that if any process failed to exit cleanly the build would immediately halt as failed; whereas if a command in the script section returns with a non-zero exit code, the build will continue to subsequent commands.

So, lets walk through your travis file

sudo: required
language: python
branches:
  only:
    - yash-v0
python:
  - "2.7"

That all looks good!

before_install:
  - pip install flake8
  - "git clone git://github.com/sugarlabs/sugar-build.git"
  - "cd sugar-build"
  - "./osbuild pull"
  - "./osbuild build"

That looks okay. However, I wonder about using the git HEAD for development; its the latest current edge stuff, but perhaps we should be testing for

a. the latest stable release with a container of fedora:23 and the released RPM package (via copr, eg https://copr.fedorainfracloud.org/coprs/samtoday/sugar/)
b. the XO-1, perhaps by using a docker container with fedora:18 and the most recent packages for it (which are sadly not the latest, eg GTK+3 is a bit old), or perhaps with a container of the OLPC OS image.

https://hub.docker.com/_/fedora/ has some tips on using fedora docker images

install: 
  - "cd activities/"
  - "git clone https://github.com/YashAgarwal/testingTravis.git edit-fonts.activity"

I would like to see us move to https://github.com/sugarlabs/edit-fonts-activity master this week

script: 
  - "flake8 edit-fonts.activity"

@samdroid-apps is this default flake8 code style profile correct?

after_success:
  - "cd .."
#  - "travis_wait 30 mvn install"
  - "./osbuild shell"
  - "python setup.py dist_xo"
  - "cd ~; ls -alFR"

This is ok, but the dist_xo command is producing a .xo binary that is then discarded; the travis file should then have a deploy section where this is uploaded to a deploy target; http://activities.sugarlabs.org is currently the main activity distribution site, and deploying there involves some authenticating (possibly to obtain a cookie) and uploading the .xo file via HTTP POST requests.

@samdroid-apps is there already any python/shell scripts for automating the posting to ASLO?

(PS, please note that when you want to make a preformatted (code) section in Github comments, you should use 3 graves instead of 1 on a line on their own, to start and end the block, rather than just 1 - which is for incline code words; and you can add (without spaces) the name of the file extension relevant to the code to get syntax highlighting, eg

```yaml
something:
- sweet
```

which looks like:

something:
- sweet
@samdroid-apps
Copy link

@samdroid-apps samdroid-apps commented Jun 14, 2016

Normal flake 8 should be fine.

I don't think there is a way to upload the bundles to ASLO automatically. Maybe mint some RPMs and put them in a CPOR?

@samdroid-apps
Copy link

@samdroid-apps samdroid-apps commented Jun 14, 2016

@davelab6, how do you set a custom docker container? I was reading the docs and I couldn't find anything. Also the announcement said:

Can I provide my own Docker containers?

The build environments we're currently using on our Docker-based setup provide the same services, programming languages and tools offered by our legacy stack. We've been keeping them on par as much as possible.

The lack of sudo does make it hard to install custom libraries and services, running them on privileged ports, or simply using apt. The question for providing your own images is a natural one.

We're not there yet in allowing you to bring your own, but it's something we have in mind for the future.

If it is not avaliable, is there an alternate CI provider that offers that? Is there something that we can self host if needed?

@samdroid-apps
Copy link

@samdroid-apps samdroid-apps commented Jun 14, 2016

@davelab6, maybe look at http://docs.gitlab.com/ce/ci/quick_start/README.html

GitLab is FLOSS (yay!) and with the runners you can setup your own and it runs it on a specific machine that you setup (like a fedora 23 docker, or an xo emulator, or even maybe an xo).

It also looks like very travis style config file :)

@samdroid-apps
Copy link

@samdroid-apps samdroid-apps commented Jun 14, 2016

Forget all those proposals. here is the best one:

SL once (thanks to @dnarvaez) had an amazing buildbot that ran all the tests. But it decayed and the server got rebooted. The docs were never found and it decayed and died :(

BUT I just stumbled upon a repo for it... with tones of docs! https://github.com/sugarlabs/sugar-buildbot

Let's get that great thing back and running again!

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jun 14, 2016

GitLab is FLOSS

Except it isn't, because the business model is making the important bits proprietary

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jun 14, 2016

Let's get that great thing back and running again!

Please do; however, I think it is likely to decay and die, and isn't worth bothering with :)

@YashAgarwal
Copy link
Contributor

@YashAgarwal YashAgarwal commented Jun 14, 2016

@davelab6 should I focus this now or rather develop the activity for the mid-term evaluation, I think it will be better to get the activity to a release stage before doing all of this

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jun 14, 2016

the announcement said:

The announcement was a long time ago :D https://docs.travis-ci.com/user/docker/ offers an example in https://github.com/travis-ci/docker-sinatra and with a quick Google search for travis docker fedora I found https://github.com/zuazo/kitchen-in-travis#official-centos-7-and-fedora-images :)

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jun 14, 2016

@YashAgarwal I agree, focus on the activity functionality :)

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 6, 2016

After 3 weeks of focus on functionality, I suggest now is a good time to return to a focus on process :)

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 9, 2016

Looking at https://travis-ci.org/sugarlabs/edit-fonts-activity flake8 is running on our entire repo, including all the modules we don't develop which don't conform to pep8, and this makes the log too long to display haha :)

In (.travis.yml) we run

flake8 EditFonts.activity

We can run

flake8 EditFonts.activity --exclude=defcon,extractor,fontTools,fontmake,robofab,ufo2ft,ufoLib

I make PR #59 to address this

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 9, 2016

I added a couple of tasks to the top:

  • Fix all errors reported by flake8
  • Set 'master' branch to be a 'protected branch' in Github to prevent regressions
@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 9, 2016

Okay, now I've set the 'master' branch to be a 'protected branch' in Github to prevent regressions like this:

screen shot 2016-07-09 at 16 17 12

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 9, 2016

This means that no one can commit to master directly any more; you HAVE to work on a branch and make a PR and wait for Travis to pass that branch before it can be merged :)

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 9, 2016

There were 939 errors reported by flake8, I've added them to the top of this issue and will tick off the ones I'm fixing as I'm fixing them, in part of PR #61

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 9, 2016

haha wow, Travis currently takes 50 minutes to complete! :) I'm going to adjust it

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 9, 2016

Okay, so, I undid branch protection to adjust the travis file to run fast, by commenting out all the osbuild stuff, and used this command to find out which files had the most flake8 errors:

$ flake8 --exclude=defcon,extractor,fontTools,fontmake,robofab,ufo2ft,ufoLib,snippets --ignore=E402 . | cut -d: -f1 | uniq -c | sort
   1 ./editfonts/objects/settings.py
  12 ./editfonts/widgets/localIcon.py
  12 ./icons/localIcon.py
  13 ./x.py
  18 ./editfonts/widgets/character_map.py
  22 ./editfonts/widgets/render_glyph.py
  23 ./editfonts/pages/welcome_page.py
  26 ./editfonts/widgets/editor_box.py
  27 ./editfonts/widgets/custom_box.py
  30 ./editfonts/widgets/form_box.py
  32 ./editfonts/objects/basefont.py
  48 ./editfonts/pages/summary_page.py
  84 ./editfonts/pages/editor_page.py
 130 ./editfonts/widgets/drag_point.py
 138 ./editfonts/pages/create_font_page.py
 197 ./editfonts/pages/manager_page.py
@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 9, 2016

So, Eli will take manager_page.py by running the flake8 command specified in .travis.yml on that file, and editing it, until it is clean:

$ flake8 --statistics --exclude=defcon,extractor,fontTools,fontmake,robofab,ufo2ft,ufoLib,snippets --ignore=E402 ./editfonts/pages/manager_page.py

And I'll do the same with create_font_page.py

And so on until they are all clean

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 10, 2016

Okay, they are all clean :)

Now when opening a pull request, Travis runs flake8 on the codebase:

screen shot 2016-07-10 at 19 06 40

And if it passes, the PR looks like this:

screen shot 2016-07-10 at 19 13 40

If it doesn't pass, the PR looks like this:

screen shot 2016-07-10 at 19 16 33

You can see the 'merge' button is greyed out; it only becomes green when the checks pass.

Also it is now not possible to push to the master branch; instead all changes to master must be done via pull request - which ensures that the codebase always passes flake8. This is the error when trying to push commits directly to master:

$ git push
Counting objects: 2, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (2/2), 226 bytes | 0 bytes/s, done.
Total 2 (delta 1), reused 0 (delta 0)
remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: Required status check "continuous-integration/travis-ci" is expected
To git@github.com:sugarlabs/edit-fonts-activity.git
 ! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to 'git@github.com:sugarlabs/edit-fonts-activity.git'

If you have commit things to master, just branch from there, push the branch to github (origin) and create a PR :)

 git push --set-upstream origin branchname
@davelab6 davelab6 self-assigned this Jul 10, 2016
@davelab6 davelab6 closed this Jul 10, 2016
@samdroid-apps
Copy link

@samdroid-apps samdroid-apps commented Jul 10, 2016

Hey @davelab6,

great work with this ci! is it possible to get this running on sugar{,-toolkit-gtk3} as well? Is this portable to other activities?

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 10, 2016

@samdroid-apps thanks :) Yes, the current .travis.yml file is really just a few lines now, so it should be portable to any python project. However, there will be a lot of errors in those codebases I expect, and probably there are other more important things to do for the codebase - like moving issues from trac to Github :)

@davelab6
Copy link
Contributor Author

@davelab6 davelab6 commented Jul 11, 2016

Also I note that when a PR isn't in sync with master, it can't be merged - looks like this:

screen shot 2016-07-11 at 01 28 09

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.