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.

Sign up for GitHub

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

Jump to bottom

Recover coverage from subprocesses during unit tests #15354

Merged
merged 2 commits into from
Mar 6, 2020
Merged

Recover coverage from subprocesses during unit tests #15354

merged 2 commits into from
Mar 6, 2020

Conversation

alalazo
Copy link
Member

@alalazo alalazo commented Mar 5, 2020

Moving pytest.ini to Spack's root folder since apparently moving to another folder during tests
causes coverage to output coverage files within lib/spack/spack/test with no coverage data.

Currently spack test can be run from any directory, but coverage will be correctly measured only if run from Spack's root folder.

@alalazo
Recover coverage from subprocesses during unit tests
f3b1fa2 
Moving pytest.ini to Spack's root folder since
apparently moving to another folder during tests
causes 'coverage' to output coverage files within
lib/spack/spack/test with no coverage data.

Currently spack test can be run from any directory,
but coverage will be correctly measured only if run
from Spack root folder.
@alalazo alalazo added tests General test capability(ies) maintainers bugfix Something wasn't working, here's a fix labels Mar 5, 2020
@alalazo
Copy link
Member Author

alalazo commented Mar 5, 2020

Can anybody stress test the spack test command locally and check that it still works as it is expected to? Otherwise I think this PR is ready to go and adds back coverage on anything tested by subprocesses during unit tests.

@alalazo alalazo added this to In progress in Spack v0.14.1 release via automation Mar 5, 2020
@alalazo alalazo mentioned this pull request Mar 5, 2020
Merged
@tgamblin
Copy link
Member

tgamblin commented Mar 5, 2020

The only downside I see here is that this means we’ll need to specify full paths (from root) to individual test files, right?

@alalazo
Copy link
Member Author

alalazo commented Mar 5, 2020

The only downside I see here is that this means we’ll need to specify full paths (from root) to individual test files, right?

Yes, that's one change. I was wondering if there are others that I missed. Also coverage is computed correctly only from Spack root, otherwise we suffer from the same problem when we move into the root with working_dir.

@scheibelp scheibelp self-assigned this Mar 5, 2020
tldahlgren
tldahlgren reviewed Mar 5, 2020
View reviewed changes
Copy link
Contributor

@tldahlgren tldahlgren left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! I ran it locally using coverage 5.0.3 and it shows coverage of build_process.

I get a lot of the following messages, which cause several of the tests to fail:

warning: No data was collected. (no-data-collected)

I'd need to re-run it to find the errors since they scrolled off my terminal window and the coverage reports don't provide that information.

Is anyone else getting these messages? Should we be disabling or investigating them?

Spack v0.14.1 release automation moved this from In progress to Reviewer approved Mar 5, 2020
tldahlgren
tldahlgren approved these changes Mar 5, 2020
View reviewed changes
Copy link
Contributor

@tldahlgren tldahlgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@scheibelp scheibelp assigned tldahlgren and unassigned scheibelp Mar 5, 2020
@alalazo
Copy link
Member Author

alalazo commented Mar 5, 2020

Is anyone else getting these messages? Should we be disabling or investigating them?

I get them too and they are the cause of the three or four changes to llnl.util.log tests here (the output was getting caught by stderr and didn't match the expected one). They seem benign to me (probably from the logger?) but I didn't investigate them deeply.

@alalazo
Copy link
Member Author

alalazo commented Mar 5, 2020

I get a lot of the following messages, which cause several of the tests to fail:

All the failing tests have been fixed in the PR - at least all the ones that are run on Travis. Is there any chance that you are running some of the skipped tests on another machine (here I'm specifically thinking of locking)?

@tldahlgren
Copy link
Contributor

tldahlgren commented Mar 5, 2020 •
edited
Loading

I get a lot of the following messages, which cause several of the tests to fail:

All the failing tests have been fixed in the PR - at least all the ones that are run on Travis. Is there any chance that you are running some of the skipped tests on another machine (here I'm specifically thinking of locking)?

Sounds like the answer is yes. I'll check. Put -v in the wrong place initially.

@tldahlgren
Copy link
Contributor

tldahlgren commented Mar 5, 2020 •
edited
Loading

UPDATE: Nvm. Ignore the errors below. I was re-directing test output to a file, which is not (yet) supported.

I get a lot of the following messages, which cause several of the tests to fail:

All the failing tests have been fixed in the PR - at least all the ones that are run on Travis. Is there any chance that you are running some of the skipped tests on another machine (here I'm specifically thinking of locking)?

The following are failing for me locally:

FAIL lib/spack/spack/test/install.py::test_nosource_pkg_install[mock_archive0]
FAIL lib/spack/spack/test/database.py::test_clear_failure_keep
FAIL lib/spack/spack/test/installer.py::test_process_external_package_module
FAIL lib/spack/spack/test/installer.py::test_process_binary_cache_tarball_none
FAIL lib/spack/spack/test/installer.py::test_process_binary_cache_tarball_tar
FAIL lib/spack/spack/test/installer.py::test_install_task_stop_iter
FAIL lib/spack/spack/test/installer.py::test_requeue_task
FAIL lib/spack/spack/test/installer.py::test_install_lock_failures
FAIL lib/spack/spack/test/installer.py::test_install_lock_installed_requeue
FAIL lib/spack/spack/test/installer.py::test_install_read_locked_requeue
FAIL lib/spack/spack/test/stage.py::TestStage::()::test_first_accessible_path

All but one of the errors appear to be related to captured output (capfd). The other issue is related a stage dir permission check (mode).

The following tests are getting module-not-imported(Module lib was never imported) followed by one or more no-data-collected warnings:

lib/spack/spack/test/cmd/install.py::test_cdash_report_concretization_error
lib/spack/spack/test/cmd/install.py::test_cdash_upload_build_error
lib/spack/spack/test/cmd/install.py::test_cdash_upload_clean_build
lib/spack/spack/test/cmd/install.py::test_cdash_upload_extra_params
lib/spack/spack/test/cmd/install.py::test_cdash_buildstamp_param
lib/spack/spack/test/cmd/install.py::test_cdash_install_from_spec_yaml
lib/spack/spack/test/cmd/env.py::test_environment_status
lib/spack/spack/test/cmd/install.py::test_cdash_auth_token
lib/spack/spack/test/container/cli.py::test_command
lib/spack/spack/test/llnl/util/log.py::test_log_python_output_with_python_stream
lib/spack/spack/test/llnl/util/log.py::test_log_subproc_output

@tldahlgren tldahlgren merged commit b2e7e7e into spack:develop Mar 6, 2020
Spack v0.14.1 release automation moved this from Reviewer approved to Done Mar 6, 2020
@tldahlgren
Copy link
Contributor

Thanks!

@alalazo alalazo deleted the maintenance/recover_coverage_information_from_subprocesses branch March 6, 2020 06:38
@adamjstewart adamjstewart mentioned this pull request Mar 13, 2020
Merged
@tgamblin tgamblin moved this from Done to Reviewer approved in Spack v0.14.1 release Mar 19, 2020
@tgamblin tgamblin moved this from Reviewer approved to Done in Spack v0.14.1 release Mar 20, 2020
tgamblin pushed a commit that referenced this pull request Mar 20, 2020
@tldahlgren @tgamblin
Recover coverage from subprocesses during unit tests (#15354)
733f9f8 
* Recover coverage from subprocesses during unit tests
likask pushed a commit to likask/spack that referenced this pull request Apr 7, 2020
Merge commit 'e2b1737a42c9c0c796671f9dd0c39f623e4c91c0' into develop_…
5473531 
…upstream_master

* commit 'e2b1737a42c9c0c796671f9dd0c39f623e4c91c0': (1343 commits)
  update CHANGELOG.md for 0.14.1
  version bump: 0.14.1
  multiprocessing: allow Spack to run uninterrupted in background (spack#14682)
  Cray bugfix: TERM missing while reading default target (spack#15381)
  Upstreams: don't write metadata directory to upstream DB (spack#15526)
  Creating versions from urls doesn't modify class attributes (spack#15452)
  bugfix: fix install_missing_compilers option bug from v0.14.0 (spack#15416)
  bugfix: installer.py shouldn't be executable (spack#15386)
  Add function replace_prefix_nullterm for use on mach-o rpaths. (spack#15347)
  ArchSpec: fix semantics of satisfies when not concrete and strict is true (spack#15319)
  suite-sparse: fix installation for v5.X (spack#15326)
  testing:  increase installer coverage (spack#15237)
  bugfix: resolve undefined source_pkg_dir failure (spack#15339)
  Bugfix: resolve StopIteration message attribute failure (spack#15341)
  Recover coverage from subprocesses during unit tests (spack#15354)
  Correct pytest.raises matches to match (spack#15346)
  bugfix:  Add dependents when initializing spec from yaml (spack#15220)
  Uniquify suffixes added to module names (spack#14920)
  bugfix: ensure proper dependency handling for package-only installs (spack#15197)
  Fix for being able to 'spack load' packages that have been renamed. (spack#14348)
  ...

# Conflicts:
#	.travis.yml
#	lib/spack/spack/modules/common.py
#	var/spack/repos/builtin/packages/mofem-cephas/package.py
#	var/spack/repos/builtin/packages/mofem-fracture-module/package.py
#	var/spack/repos/builtin/packages/mofem-users-modules/package.py
#	var/spack/repos/builtin/packages/python/package.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tldahlgren tldahlgren tldahlgren approved these changes

@tgamblin tgamblin Awaiting requested review from tgamblin

@adamjstewart adamjstewart Awaiting requested review from adamjstewart

@becker33 becker33 Awaiting requested review from becker33

@scheibelp scheibelp Awaiting requested review from scheibelp

Assignees

@alalazo alalazo

@tldahlgren tldahlgren

Labels
bugfix Something wasn't working, here's a fix maintainers tests General test capability(ies)
Projects
No open projects
Spack v0.14.1 release
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@alalazo @tgamblin @tldahlgren @scheibelp