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

Minor cleanups to integration tests #6734

Merged
merged 2 commits into from Nov 8, 2018
Merged

Minor cleanups to integration tests #6734

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e92ab86
Add clean-alls where needed and helpful comments
Nov 7, 2018
90c145a
Clean up unneeded comments and int test
Nov 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 0 additions & 8 deletions tests/python/pants_test/backend/python/tasks/test_ctypes_integration.py
View file
Open in desktop
Expand Up @@ -122,21 +122,13 @@ def test_ctypes_third_party_integration(self):
self.assert_success(pants_run)
self.assertIn('Test worked!\n', pants_run.stdout_data)

# Test cached run.
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

To test a cached run, you could use something like: https://github.com/pantsbuild/pants/blob/master/tests/python/pants_test/backend/jvm/tasks/jvm_compile/test_dep_exports_integration.py#L35

ie, run pants twice in the same temporary workdir.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack, will add.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that test checking for a cached run by checking whether the output contains the target spec (the last line of that method)? Where is that message coming from? How does that technique compare to the use of self.mock_buildroot() as used in another method also named test_invalidation()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stuhood after reviewing the conan code further, I see that this test is unnecessary because really the only thing that needs to be tested is whether the invalidation check does what it is supposed to.

pants_run = self.run_pants(
command=['-q', 'run', self._binary_target_with_third_party]
)
self.assert_success(pants_run)
self.assertIn('Test worked!\n', pants_run.stdout_data)

def test_pants_native_source_detection_for_local_ctypes_dists_for_current_platform_only(self):
"""Test that `./pants run` respects platforms when the closure contains native sources.

To do this, we need to setup a pants.ini that contains two platform defauts: (1) "current" and
(2) a different platform than the one we are currently running on. The python_binary() target
below is declared with `platforms="current"`.
"""
# Clean all to rebuild requirements pex.
command = [
'run',
'testprojects/src/python/python_distribution/ctypes:bin'
Expand Down