A lot of good ideas here. Some comments:

Package tests

I suspect that implementing these tests could uncover many issues that
could take a while to fix.

• prevents problems like Remove url_for_version as it breaks spack info #1503
  Remove url_for_version as it breaks spack info #1503

Code review can also catch this, if we understand that packages shouldn't
be accessing the Internet on their own.

• • spack fetch
  • try fetching every single version to make sure they are still
    available
  • checks that url_for_version works for every version

This won't work for Spack packages that:
a) Come from non-public repositories in some cases (modele).
b) Require manual download (galahad, java)

• spack spec
  • run for every possible set of variants

Running time will be exponential in the number of variants. This could
cause problems.

I would like to see additional Package tests, trying to build each
package. We obviously cannot build every variant of every package, with
every set of dependencies. But we should be able to set up "standard
versions" of each package that Spack tries to build, and reports on
success/failure on various OS releases. At the minimum, testing should try
to build the default (no-variant) version of every package.

Failure of the build test on a machine should not a priori be a reason to
reject a package. But it would be really cool if we can look at a
particular package, and see what machines it has / has not successfully
built in the past. Kind of like this:

https://appdb.winehq.org/objectManager.php?sClass=version&iId=3665

Documentation tests

If you try building Spack's documentation, you'll notice that there are
several warning and error messages. These messages should cause the test to
fail. This prevents problems like #1140
#1140 (oops, my fault). We want
developers to add documentation as frequently as possible, even if they
don't know how to test their changes or aren't very familiar with
reStructuredText.

As a part of this PR, I will attempt to resolve all current warning and
error messages, so as not to repeat past mistakes with flake8 integration.

Great!

There isn't any way to automatically deploy new documentation to
http://software.llnl.gov/spack/index.html when a documentation changing
PR is accepted, is there???

If you re-deploy every night, that's close enough.

Good ideas. I agree that we should test things before merging, at lest on a few major Unixes + GCC. Of course MacOS + clang + gfortran would also be great.

My understanding that the only issue could be with

spack install --run-tests

which may take some time (e.g. trilinos build). To make it faster i think we need binaries. So if someone changes something in trilinos, it should be possible to fetch pre-build dependencies (gcc, openmpi, openblas, etc) and then try to build the modified version of the package. Otherwise building the whole DAG from scratch will likely take a huge amount of time for big packages like trilinos and dealii.

@davydden Yes, building the entire DAG could be time consuming. But how many of these PR's do you want merged more quickly than it could possibly take to build? OpenMPI should only take ~20 minutes? OpenBLAS is more like ~45 minutes? It depends on the compiler. Either way, if it takes less than half a day, I don't think it will adversely affect PRs. If we want to install multiple variants with multiple compilers on multiple OSes, then maybe it will become overkill. spack install was definitely the biggest stretch for this PR.

But how many of these PR's do you want merged more quickly than it could possibly take to build?

i would prefer to test everything before merging, thereby reduce chance of braking things.

If we want to install multiple variants with multiple compilers on multiple OSes

I think building default variant is indeed a good compromise here. We can start with one OS only, say, Ubuntu 14.04+GCC.

i would prefer to test everything before merging, thereby reduce chance of braking things.

I agree, and I hope people will still test things locally. But I would feel safer if Travis tried installing everything on a bare-bones Ubuntu to catch missing dependencies and OS differences. Just because something builds on osx doesn't mean it builds on Ubuntu. Also, sometimes people will add a new version to Spack without actually making sure that it installs. This will catch that too.

Btw, I agree with your Ubuntu/GCC/default-variants compromise, unless someone else really wants to test everything. I would love to see osx testing, but Travis doesn't seem to support it...

For some reason, Git thinks I deleted run-flake8 and created a new file called run-flake8-tests even though I ran git mv run-flake8 run-flake8-tests. Does anyone know if there's a way to fix this? Otherwise, the history of that file will be lost.

maybe a github issue? Does it also say the file is deleted in git status?

@adamjstewart: git rebase -i is your friend. Get to know git reflog though so you can recover from screwups :)

I think git determines whether a file moved based on the file's content. Did you make substantial changes to the file in the same commit? If so, does git change its opinion if there is one rename commit, and then one content-changing commit?

I agree with most of the things said here, except

Only run spack test if core Spack libraries have been modified.

I don't see the benefit in this. Core should always be working and testing it doesn't take a long time.

To state it differently, if you don't modify spack core and you have red light on it then it's very likely you spotted an heisenbug and those kind of things are better spotted early.

For spack install <something> tests we may annotate them within comments directly in the package.py files, i.e. something like :

# <architecture>:
#   hdf5~cxx+mpi+szip ^ openmpi % gcc
#   hdf5~cxx+mpi+szip ^ mpich %gcc
#   hdf5+cxx~mpi+szip % intel
#   <other specs>

What do you think about that ?

@eschnett I did make substantial changes to the file. I tried separating the file changes and the file rename (with git mv) into separate commits but that didn't help.

@davydden It says the file was renamed in git status before the commit but after the commit it says it was deleted.

@alalazo

I agree with most of the things said here, except

Only run spack test if core Spack libraries have been modified.

I don't see the benefit in this. Core should always be working and testing it doesn't take a long time.

I'm not sure how I feel about running spack test solely for the purpose of trying to catch Heisenbugs. You're right, it doesn't take long (6 min 30 sec on our cluster), but adding package specific testing is just as likely to catch Heisenbugs. Also, won't most people just force re-run Travis to get past Heisenbugs anyway?

In my mind, the point of Travis is not to test Spack, but to test a PR. If the PR only affects documentation and the documentation builds without any errors, Travis should send us the green light and let us know that the PR is safe to merge.

Also, won't most people just force re-run Travis to get past Heisenbugs anyway?

If most people like bad practices then I would try to make their life harder, not easier 😄

If the PR only affects documentation and the documentation builds without any errors, Travis should send us the green light and let us know that the PR is safe to merge.

Fine, but if it involves a package it should still try to run spack test, at least imho

After tracking down most of the bugs in the documentation, I'm starting to realize that we should always be running documentation tests. The documentation doesn't just come from lib/spack/docs. It also comes from the description in each package.py and docstrings from across Spack's core libraries. It is dependent on commands like spack package-list. Documentation has gone missing just because etc/spack/packages.yaml moved to etc/spack/defaults/packages.yaml. Since something as simple as adding a new package to Spack can introduce warnings and even errors into the documentation, I've updated run-doc-tests to always run, regardless of what files were edited.

@adamjstewart: I agree with most everything. Thanks! This is awesome.

I have a few suggestions:

1. Like @alalazo, I think running spack test for everything is a good idea. Not all of the test work only on mock packages; there are many (like package_sanity) that ensure things about all the builtin packages. It would be good to run those. Also, I would like to know if something somehow comes up in a package that breaks spack.Spec. For example, Spack is not particularly good at cycle detection right now, and that can cause explosions (remember git and intltool?).
2. The package tests are not all going to run on Travis -- they'll time out. I also think if we ran all possible variant tests on Travis, we might put them out of business. So it would be good it we could work together on this one. At LLNL, I've got some money I can spend on time in AWS, and we have a guy starting soon who I would like to work on this stuff. Basically I'd like LLNL to run a build bot that runs the package-specific tests in AWS and pings github if they pass. NERSC has told me they're willing to run tests on their systems, as well. And we're looking at how we might sandbox tests on Linux and BG/Q clusters at LLNL itself.
3. To avoid exponential testing time, I think we should start looking at creating some concretization defaults for, say, stable, unstable, etc. This would be sort of like preferred=True, but the user would have more control, and you could pick which stack you want (kind of like how you can do with debian and other distros). This would give us a frontier in the build space where we could do a reasonable set of checks, but it would avoid the nasty exponential aspects of testing all the variants. stable and unstable could potentially contain a few popular variant combinations -- I don't mind doing a very large number of tests, but I'd like to keep it tractable.

Thoughts?

I always figured we'd pick and choose which variant combos we test. Start
off with testing just the default variant, and then maybe add a few more
over time.

-- Elizabeth

On Sun, Aug 28, 2016 at 10:55 PM, Todd Gamblin [email protected]
wrote:

@adamjstewart https://github.com/adamjstewart: I agree with most
everything. Thanks! This is awesome.

I have a few suggestions:

Like @alalazo https://github.com/alalazo, I think running spack test
for everything is a good idea. Not all of the test work only on mock
packages; there are many (like package_sanity) that ensure things
about all the builtin packages. It would be good to run those. Also, I
would like to know if something somehow comes up in a package that breaks
spack.Spec. For example, Spack is not particularly good at cycle
detection right now, and that can cause explosions (remember git and
intltool?).
2.

The package tests are not all going to run on Travis -- they'll time
out. I also think if we ran all possible variant tests on Travis, we
might put them out of business. So it would be good it we could work
together on this one. At LLNL, I've got some money I can spend on time in
AWS https://aws.amazon.com, and we have a guy starting soon who I
would like to work on this stuff. Basically I'd like LLNL to run a build
bot that runs the package-specific tests in AWS and pings github if they
pass. NERSC has told me they're willing to run tests on their systems, as
well. And we're looking at how we might sandbox tests on Linux and BG/Q
clusters at LLNL itself.
3.

To avoid exponential testing time, I think we should start looking at
creating some concretization defaults for, say, stable, unstable, etc.
This would be sort of like preferred=True, but the user would have
more control, and you could pick which stack you want (kind of like how you
can do with debian and other distros). This would give us a frontier in the
build space where we could do a reasonable set of checks, but it would
avoid the nasty exponential aspects of testing all the variants.
stable and unstable could potentially contain a few popular variant
combinations -- I don't mind doing a very large number of tests, but I'd
like to keep it tractable.

Thoughts?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1576 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd5-25X2ToMW4XMPVnrjfRTyaMe_9ks5qkkomgaJpZM4JqDwH
.

btw, if you rebase on develop, you will have this commit i think. So maybe there is no need to cherry-pick?

BTW... I would recommend trying the SmartGit GUI. It's a really nice way
from moving from "what command do I need" to a more holistic understanding
if how git and my git repos work.

On Tue, Aug 30, 2016 at 3:33 PM, Massimiliano Culpo <
[email protected]> wrote:

Do you know why travis is not triggered correctly ? I have the same issue
in #107 #107 ...

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1576 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd-yDZvDZR_m2fH3F1P7s5OsAx377ks5qlIWIgaJpZM4JqDwH
.

@davydden I did rebase off of develop but it wasn't there. I'll try again. Worst comes to worst, I'll just copy and paste into Vim 😄

@alalazo @tgamblin Alright, I undid any changes to trailing triple quotes in docstrings. If you catch any that I missed, let me know.

@davydden Turns out I had to run this first:

$ git fetch upstream features/travis

Turns out I had to run this first:

$ git fetch upstream features/travis

surprising.

git fetch upstream should have updated all branches. Anyway, glad that it is sorted out.

You're probably right. But I was being obstinate and was convinced that it should've been in upstream develop.

All documentation bugs are now resolved!!! 🎉

Last remaining thing preventing this PR from being merged. In run-doc-tests, I tried adding $SPACK_ROOT/bin to the PATH. The documentation builds locally, but when run on Travis, it crashes and says it can't find the spack command. Any ideas?

basic_usage.rst:40: ERROR: Command u'spack list' failed: [Errno 2] No such file or directory

@adamjstewart you were right, the commit was in features/travis. You can see it under the commit message here 15bb41e . Sorry, did not pay enough attention and just assumed it's something that is already committed to develop.

@adamjstewart: all Travis bugs fixed and merged. Thanks!

I'm working on the merge. It doesn't look too bad, I'll post a PR soon,
showing the results.

On Tue, Aug 30, 2016 at 11:44 PM, Todd Gamblin [email protected]
wrote:

@adamjstewart https://github.com/adamjstewart: all Travis bugs fixed
and merged. Thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1576 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cdyDeNQS4VPYDwyjONBSGbtKZ1x0Uks5qlPiIgaJpZM4JqDwH
.

@citibeth: Thanks! To answer your earlier question, we're planning to not worry about the online docs for tomorrow, so don't put yourself out. Thanks for all the contributions!

@citibeth @davydden @adamjstewart: I just pushed a couple more fixes (mostly in conf.py) to get the readthedocs build working. So any changes will be visible at http://spack.rtfd.io or http://spack.readthedocs.io

🎉

OK... well, I think the structure of what I'm doing to the docs should be
becoming apparent by this point, which should help refering to the docs
from the tutorial.

1. Under "Getting Started," I now have a much more detailed guide on
   installing and setting up Spack. Spack is easy to set up, you just
   download and run... unless you're running on a real supercomputer with
   weird compilers, hand-built modules, ancient versions of SSL/binutils,
   custom MPI builds, etc. Or you're running on a Cray. The greatly expanded
   getting_started.rst should cover all this, and more! Once it's up on
   readthedocs, it will also be a page I can refer people to from my
   application installation instructions (rather than rewriting the Spack
   install guide each time).

Some material more appropriate to this new expanded getting_started was
moved from the catch-all basic_usage. I see basic_usage as a reference
manual of features you can use once you have Spack set up.

1. I created a new spack_workflows. The idea of this is to show how things
   can all come together with Spack not just to build software, but also to
   use it in useful ways. There are a number of different patterns people
   have implemented. The aim is to describe them all in this section and give
   the pros and cons of each.

The material in spack_workflows is absolutely something we should consider
including or linking to from the tutorial. I'll finish up the first draft
of it on Wed.

-- Elizbaeth

On Wed, Aug 31, 2016 at 12:11 AM, Todd Gamblin [email protected]
wrote:

@citibeth https://github.com/citibeth: Thanks! To answer your earlier
question, we're planning to not worry about the online docs for tomorrow,
so don't put yourself out. Thanks for all the contributions!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1576 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd1-0d8DD8G7kDh1p9tgowxJ68pC2ks5qlP7dgaJpZM4JqDwH
.