ENH: add tests to qastools recipe

[mrakitin]

The motivation is an hour spent for tracking down this missing __init__.py.

Besides that I suspect that our Docker image does not perform testing, as I see those messages when built locally (without pushing):

...
Fixing permissions
Compressing to /tmp/tmpediah7c1/qastools-3.0.1qas-py36_1.tar.bz2
Importing conda-verify failed.  Please be sure to test your packages.  conda install conda-verify to make this message go away.
WARNING:conda_build.build:Importing conda-verify failed.  Please be sure to test your packages.  conda install conda-verify to make this message go away.
...

However, when I run a build directly in the container with conda build ., I get this:

...
Fixing permissions
Compressing to /tmp/tmpian4ismf/qastools-3.0.1qas-py36_1.tar.bz2
TEST START: /conda/conda-bld/linux-64/qastools-3.0.1qas-py36_1.tar.bz2
Adding in variants from /tmp/tmpx29gahe2/info/recipe/conda_build_config.yaml
INFO:conda_build.variants:Adding in variants from /tmp/tmpx29gahe2/info/recipe/conda_build_config.yaml
Renaming work directory,  /conda/conda-bld/qastools_1554843658637/work  to  /conda/conda-bld/qastools_1554843658637/work_moved_qastools-3.0.1qas-py36_1_linux-64
Solving environment: ...working... done
...

[mrakitin]

Attn @NSLS-II/dama:

After some investigation I noticed that the testing was missed after the #559 PR was merged. I compared the behavior of our build.py script with what conda build . does. The latter one uses this code in the CLI. The difference between the api.build and build.build is that the former one calls conda_build.build.build_tree(), which in turn performs the testing, whereas the latter one does not do any testing except the verification (it's a separate thing, wrongly pointed out in the summary of this PR). That's why I had to change the place where conda's build is imported in our build.py: conda_build.build -> conda_build.api. The testing started to be performed after this change.

We may need to rebuild the packages pushed after March 1, 2019 (the date #559 was merged). As the tests weren't performed, we may have some broken packages, but I am not sure. What do people think?

[danielballan]

Thinking of resources and priorities, I am comfortable addressing this as it comes up rather than rebuilding everything since March 1.

[mrakitin]

Thanks for the reply, @danielballan. Another important issue is the Debian 7, which we cannot reliably rebuild due to archiving the package repositories. See more at NSLS-II/debian-with-miniconda#15.

[leofang]

After some investigation I noticed that the testing was missed after the #559 PR was merged. I compared the behavior of our build.py script with what conda build . does. The latter one uses this code in the CLI. The difference between the api.build and build.build is that the former one calls conda_build.build.build_tree(), which in turn performs the testing, whereas the latter one does not do any testing except the verification (it's a separate thing, wrongly pointed out in the summary of this PR). That's why I had to change the place where conda's build is imported in our build.py: conda_build.build -> conda_build.api. The testing started to be performed after this change.

Oh, so sorry for not including tests in my PR and for wasting your time on tracking down the issue, @mrakitin!

Regarding using Conda's public API, if I remember correctly there were issues where rendering recipe with multiple input/output variants, so in dfccbde I reverted back to use solely Conda's internal APIs. But perhaps using conda_build.api.build is irrelevant of those issues. If we want we could also call the internal API for testing and keep conda_build.build.build intact.

[mrakitin]

No problem, @leofang, I'm glad we have found this omission. I suppose as the CLI's build via conda_build.api.build works fine for multi-variant recipes, it should just work for our code as well. It wasn't wasting of the time, but rather a useful learning of the conda-build's internals - I have more understanding now.

This PR needs a new release of qastools to proceed, will tag it shortly.

[mrakitin]

Here is a list of packages built since March 1, 2019, in case we may want to rebuild them:

02:23 # git log --pretty=format: --name-only --since="2019-03-01" | grep recipes-tag/ | cut -d/ -f2 | sort -u
04-bm-xfm-collection
05-id-srx-analysis
05-id-srx-collection
07-bm-qas-collection
08-id-iss-analysis
08-id-iss-collection
11-id-chx-analysis
11-id-chx-collection
12-id-smi-analysis
12-id-smi-collection
analysis
bluesky
caproto
collection
databroker
event-model
fabio
grid-strategy
hdf5-lz4
isstools
lz4-c
matplotlib
mayavi
mpich
mpl-modest-image
nslsii
openmpi
ophyd
pychx
pystackreg
pyxrf
qastools
qasxas
scikit-beam
shed
suitcase-csv
suitcase-json-metadata
suitcase-jsonl
suitcase-specfile
suitcase-tiff
suitcase-utils
tifffile
toolchain
xas
xpdan
xpdtools
xpdview
xray-vision
xraylib

[leofang]

I just took another look at conda_build.api.build and I am not sure if it's better to call it directly. Internally, it calls conda_build.build.build_tree, which does a whole lot of work (e.g., resolving dependencies) that we already did before calling CondaBuild (i.e., conda_build.build.build). In addition, it tests not only the built package itself, but also all "downstream" packages --- not sure what it means but I don't think we need this behavior, do we? See https://github.com/conda/conda-build/blob/fc22ab8c161a6c79ed3d48fab0ffc954a5fa61c2/conda_build/build.py#L2282-L2323

Have you tested this change with the MPI chain and Matplotlib? Are they working fine? If so, we can merge it and worry about the redundant work later when the build system is reworked in the summer.

Alternatively, I think we could just call conda_build.build.test ourselves after the CondaBuild block

lightsource2-recipes/scripts/build.py

Lines 537 to 553 in ad8b4b8

	try:
	stats = {} # statistics for conda build internal; not needed
	# at this point meta should know the pinned numpy version
	CondaBuild(meta, stats=stats)
	except Exception as e:
	build_or_test_failed.append(build_name)
	logger.error(e)
	#message_slack(message, username, is_error=True)
	if not allow_failures:
	sys.exit(1)
	finally:
	# observation: older versions of conda build do not take care of the
	# temporary directories properly. In such cases we clean it up manually.
	# this is mainly to work with our customized docker image
	work_dir = meta.config.work_dir
	if work_dir.startswith("/conda/conda-bld/"):
	shutil.rmtree("/conda/conda-bld/work/")

        try:
            conda_build.build.test(meta, config=meta.config.copy(), stats=stats)
        except:
            # report error, skip this package, and proceed to the next one?

which is similar to how it is used in conda_build.build.build_tree, see https://github.com/conda/conda-build/blob/fc22ab8c161a6c79ed3d48fab0ffc954a5fa61c2/conda_build/build.py#L2277-L2281

[mrakitin]

Thanks @leofang. Yes, I tried to add the step for testing explicitly, but then decided to follow what conda-build does. I haven't tried it with mpi or matplotlib yet. Will test and let you know.

[leofang]

right, they are the two most weird recipes we have if I'm not mistaken, so should be good enough test cases 😄

[leofang]

btw I see that the recipe is updated in #650. It'd be nice to rename this PR before merging for future reference.

[mrakitin]

Will do and will crean up this PR from the QAS-related things.

[ke-zhang-rd]

I will leave this open and won't merge for 2019C3.0.

[mrakitin]

This work can be now done via a corresponding feedstock in https://github.com/nsls-ii-forge. Leaving it open for now before we reach the package there.

[mrakitin]

Power-cycled to enable CI.

[mrakitin]

Power-cycled to pick up the new TravisCI config from #793.

[mrakitin]

I don't understand why TravisCI cannot get all changed files right in https://travis-ci.org/NSLS-II/lightsource2-recipes/jobs/587514755#L272-L275.

Locally I can get the recipes-tag/qastools/meta.yaml file, but not on Travis:

$ git branch -avv
  add-dashboard                                    831e043 [upstream/add-dashboard] Added dashboard
  add-hxntools-to-srx-analysis-meta                d22e22b [origin/add-hxntools-to-srx-analysis-meta] Add hxntools to the SRX analysis metapackage
* enh_qastools_recipe_tests                        ad8b4b8 [origin/enh_qastools_recipe_tests] Commented out some heavy imports
  master                                           2505d64 [origin/master] databroker v1.0.0a5 (#792)
  remotes/origin/HEAD                              -> origin/master
  remotes/origin/add-hxntools-to-srx-analysis-meta d22e22b Add hxntools to the SRX analysis metapackage
  remotes/origin/enh_qastools_recipe_tests         ad8b4b8 Commented out some heavy imports
  remotes/origin/master                            2505d64 databroker v1.0.0a5 (#792)
  remotes/upstream/add-dashboard                   831e043 Added dashboard
  remotes/upstream/master                          0b74b20 CI: fix git diff command (#793)

$ git diff --name-only upstream/master...HEAD
recipes-tag/qastools/meta.yaml
scripts/build.py

$ git remote -v
origin	https://github.com/mrakitin/lightsource2-recipes (fetch)
origin	https://github.com/mrakitin/lightsource2-recipes (push)
upstream	https://github.com/NSLS-II/lightsource2-recipes (fetch)
upstream	https://github.com/NSLS-II/lightsource2-recipes (push)

@danielballan, any thoughts?

[mrakitin]

Power-cycled to pick up #794.

[mrakitin]

I removed the commit touching scripts/build.py so that this PR only addresses QAS-related recipe, and does not block the merging nor introduces a potential failure on freyja. As we now have a proper CI enabled for this repo, and we are moving to the feedstocks at @nsls-ii-forge anyway, I don't think we are going to invest any more efforts in improving the existing scripts for freyja.

[mrakitin]

Power-cycled to also pick up #795.

[mrakitin]

Closing it as the identical update to qastools was done via #650, and the building system issue was cut out from this PR.

It's good to know the build is passing now: https://travis-ci.org/NSLS-II/lightsource2-recipes/builds/588175819.