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.

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

add integration test for invalidation of ctypes c/c++ sources #6801

Merged
merged 1 commit into from Nov 27, 2018

Conversation

Projects
None yet
3 participants
@cosmicexplorer
Copy link
Contributor

cosmicexplorer commented Nov 22, 2018

Problem

We don't have any testing of invalidation or caching with the C/C++ => python_dist() workflow. @CMLivingston may have spotted a caching bug in the wild, but we're not sure about that, and in the process of investigating we realized we didn't have any testing around this.

Solution

  • Add an integration test in which we modify a C++ source file in a mock buildroot in order to see that the output of a python_binary() changes.

@cosmicexplorer cosmicexplorer requested review from stuhood and CMLivingston Nov 22, 2018

def test_invalidation_ctypes(self):
"""Test that the current version of a python_dist() is resolved after modifying its sources."""
with temporary_dir() as tmp_dir:
with self.mock_buildroot(

This comment has been minimized.

@stuhood

stuhood Nov 26, 2018

Member

Rather than mocking the buildroot, generally integration tests mock only the workdir while keeping the same buildroot.

That looks like with self.temporary_workdir() as workdir:, and no adjustment of the buildroot.

This comment has been minimized.

@CMLivingston

CMLivingston Nov 26, 2018

Contributor

Is this cool even when the tests modify testproject source files?

This comment has been minimized.

@stuhood

stuhood Nov 26, 2018

Member

Oh. Missed that.

If modifying files the mock buildroot is not a bad idea. There is at least one @contextmanager to assist with in-place-edit-and-then-restore usecases, but creating the mock buildroot is probably superior.

Thanks.

This comment has been minimized.

@cosmicexplorer

cosmicexplorer Nov 27, 2018

Contributor

(I think I may have introduced that contextmanager you're referring to and then removed it unless I'm mistaken -- the use of the mock_buildroot was in response to a comment @wisechengyi made on another PR of mine a while ago, I forget exactly which right now)

@CMLivingston
Copy link
Contributor

CMLivingston left a comment

LGTM, CI failure seems unrelated.

@cosmicexplorer cosmicexplorer merged commit e2c25e6 into pantsbuild:master Nov 27, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

cosmicexplorer added a commit that referenced this pull request Nov 27, 2018

add integration test for invalidation of ctypes c++ sources (#6801)
### Problem

We don't have any testing of invalidation or caching with the C/C++ => python_dist() workflow. @CMLivingston may have spotted a caching bug in the wild, but we're not sure about that, and in the process of investigating we realized we didn't have any testing around this.

### Solution

- Add an integration test in which we modify a C++ source file in a mock buildroot in order to see that the output of a `python_binary()` changes.

cosmicexplorer added a commit that referenced this pull request Nov 29, 2018

add integration test for invalidation of ctypes c++ sources (#6801)
### Problem

We don't have any testing of invalidation or caching with the C/C++ => python_dist() workflow. @CMLivingston may have spotted a caching bug in the wild, but we're not sure about that, and in the process of investigating we realized we didn't have any testing around this.

### Solution

- Add an integration test in which we modify a C++ source file in a mock buildroot in order to see that the output of a `python_binary()` changes.

cosmicexplorer added a commit that referenced this pull request Dec 1, 2018

add integration test for invalidation of ctypes c++ sources (#6801)
### Problem

We don't have any testing of invalidation or caching with the C/C++ => python_dist() workflow. @CMLivingston may have spotted a caching bug in the wild, but we're not sure about that, and in the process of investigating we realized we didn't have any testing around this.

### Solution

- Add an integration test in which we modify a C++ source file in a mock buildroot in order to see that the output of a `python_binary()` changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment