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

testing: increase installer coverage #15237

Merged
merged 34 commits into from
Mar 12, 2020
Merged

testing: increase installer coverage #15237

merged 34 commits into from
Mar 12, 2020

Conversation

tldahlgren
Copy link
Contributor

@tldahlgren tldahlgren commented Feb 26, 2020 •
edited
Loading

This PR is dedicated to increasing coverage of the new installer.py module.

@tldahlgren tldahlgren added WIP tests General test capability(ies) labels Feb 26, 2020
@tldahlgren tldahlgren self-assigned this Feb 26, 2020
@tldahlgren tldahlgren mentioned this pull request Feb 27, 2020
Merged
@tldahlgren
Copy link
Contributor Author

Note d5d602f is just to determine if coverage combine is needed to cover the forked build_process in installer._install_task, which appears to have no coverage despite the fork call being made.

@tldahlgren
Copy link
Contributor Author

Reset status to see if that will re-run the tests.

@tldahlgren tldahlgren closed this Mar 2, 2020
@tldahlgren tldahlgren reopened this Mar 2, 2020
@tldahlgren
Copy link
Contributor Author

Interesting. Looks like explicitly adding coverage combine to run-unit-tests causes a fatal error:

No data to combine
The command "share/spack/qa/run-$TEST_SUITE-tests" exited with 1.

So reverted.

@tldahlgren
Copy link
Contributor Author

FYI. As of commit 58a641e, installer.py has 90.61% coverage (i.e., everything but the forked bulid_process.

@tldahlgren
Copy link
Contributor Author

tldahlgren commented Mar 3, 2020 •
edited
Loading

Very strange. Coverage for eeceb04 at https://codecov.io/gh/spack/spack shows counts on comment and lines -- see lines starting 668 -- and no coverage for tests that are passing (e.g., lines ~1101 exercised by test_install_task_stop_iter).

@tldahlgren
Copy link
Contributor Author

tldahlgren commented Mar 10, 2020 •
edited
Loading

@tldahlgren See #15354

That was great! It revealed most of build_process was already being covered.

@tldahlgren tldahlgren added this to In progress in Spack v0.14.1 release via automation Mar 10, 2020
@tldahlgren tldahlgren moved this from In progress to Review in progress in Spack v0.14.1 release Mar 10, 2020
alalazo
alalazo reviewed Mar 11, 2020
View reviewed changes
Copy link
Member

@alalazo alalazo 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.

Leaving a few minor comments, tests look good to me. Can you rebase the PR?

lib/spack/spack/installer.py Outdated Show resolved Hide resolved
lib/spack/spack/test/install.py Outdated Show resolved Hide resolved
lib/spack/spack/test/install.py Outdated Show resolved Hide resolved
lib/spack/spack/test/install.py Outdated Show resolved Hide resolved
lib/spack/spack/test/installer.py Outdated Show resolved Hide resolved
@tldahlgren
Copy link
Contributor Author

@alalazo I addressed your feedback and we're getting ci test and coverage results but the builds are complaining about fetching archives. Should I try closing and re-opening?

@tldahlgren tldahlgren closed this Mar 12, 2020
Spack v0.14.1 release automation moved this from Review in progress to Done Mar 12, 2020
@tldahlgren tldahlgren reopened this Mar 12, 2020
Spack v0.14.1 release automation moved this from Done to In progress Mar 12, 2020
@tldahlgren tldahlgren closed this Mar 12, 2020
Spack v0.14.1 release automation moved this from In progress to Done Mar 12, 2020
@tldahlgren tldahlgren reopened this Mar 12, 2020
Spack v0.14.1 release automation moved this from Done to In progress Mar 12, 2020
@tldahlgren
Copy link
Contributor Author

tldahlgren commented Mar 12, 2020 •
edited
Loading

@alalazo I addressed your feedback and we're getting ci test and coverage results but the builds are complaining about fetching archives. Should I try closing and re-opening?

Well, they were there -- patch at 100%, project at -.16% -- until I opened and closed the PR twice in a row. Maybe it will reset them from CI failed once the CI tests finish. T.T

Spack v0.14.1 release automation moved this from In progress to Reviewer approved Mar 12, 2020
@alalazo
Copy link
Member

alalazo commented Mar 12, 2020

@tldahlgren The PR looks good to me. There were issues with our CI due to (I think) some change in Github apt repositories that were solved in #15458. If you can do a final update to this PR I'll merge it as soon as it goes green.

@tldahlgren
Copy link
Contributor Author

@tldahlgren The PR looks good to me. There were issues with our CI due to (I think) some change in Github apt repositories that were solved in #15458. If you can do a final update to this PR I'll merge it as soon as it goes green.

Thanks @alalazo

@tldahlgren
Copy link
Contributor Author

@alalazo They are passing now.

@alalazo alalazo merged commit f2a13af into spack:develop Mar 12, 2020
Spack v0.14.1 release automation moved this from Reviewer approved to Done Mar 12, 2020
@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
testing: increase installer coverage (#15237)
1c8f792
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

@alalazo alalazo alalazo approved these changes

@becker33 becker33 Awaiting requested review from becker33

Assignees

@tldahlgren tldahlgren

Labels
impact-high 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
2 participants
@tldahlgren @alalazo