Fix our git tests to actually test that we pushed #1532
Merged
+114
−23
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Our git tests aren't actually testing very much at the moment. We can replace the entire body of `commit_and_push` with `Ok(())`, and the test suite would pass. Right now all our tests do is check that we manipulated files in a given directory, which is more of an implementation detail than anything else. Aside from not actually testing what we want, these tests also break with the setup I want to use for async index updates. Once the index updates are off the web server, there's no reason we need to keep a local checkout of the index. We can afford to just do a full clone every time, which simplifies our code. Once we do that though, our tests can't observe the directory the commit was made in. Pushing to a remote repository doesn't actually update the files you changed on disk. Instead, to observe the changes that were committed, we create a new clone of the repo and look at the files there. Almost all of the tests are easy to change to this structure. One of our tests manipulated a file in the location that it expected the server to manipulate though. We'd have to push that for the test to be legit. We could write a `commit_and_push` function for our tests, but that function will be a pain in the ass. Instead we can just go though a full crate upload twice to test what we want.
sgrif
force-pushed
the
sg-fix-git-tests
branch
from
3d8ecac to
93b42fb
Compare
bors-voyager bot
added a commit
that referenced
this pull request
Oct 24, 2018
1532: Fix our git tests to actually test that we pushed r=jtgeibel a=sgrif Our git tests aren't actually testing very much at the moment. We can replace the entire body of `commit_and_push` with `Ok(())`, and the test suite would pass. Right now all our tests do is check that we manipulated files in a given directory, which is more of an implementation detail than anything else. Aside from not actually testing what we want, these tests also break with the setup I want to use for async index updates. Once the index updates are off the web server, there's no reason we need to keep a local checkout of the index. We can afford to just do a full clone every time, which simplifies our code. Once we do that though, our tests can't observe the directory the commit was made in. Pushing to a remote repository doesn't actually update the files you changed on disk. Instead, to observe the changes that were committed, we create a new clone of the repo and look at the files there. Almost all of the tests are easy to change to this structure. One of our tests manipulated a file in the location that it expected the server to manipulate though. We'd have to push that for the test to be legit. We could write a `commit_and_push` function for our tests, but that function will be a pain in the ass. Instead we can just go though a full crate upload twice to test what we want. Co-authored-by: Sean Griffin <[email protected]>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Our git tests aren't actually testing very much at the moment. We can
replace the entire body of
commit_and_pushwithOk(()), and thetest suite would pass. Right now all our tests do is check that we
manipulated files in a given directory, which is more of an
implementation detail than anything else.
Aside from not actually testing what we want, these tests also break
with the setup I want to use for async index updates. Once the index
updates are off the web server, there's no reason we need to keep a
local checkout of the index. We can afford to just do a full clone every
time, which simplifies our code. Once we do that though, our tests can't
observe the directory the commit was made in.
Pushing to a remote repository doesn't actually update the files you
changed on disk. Instead, to observe the changes that were committed, we
create a new clone of the repo and look at the files there. Almost all
of the tests are easy to change to this structure. One of our tests
manipulated a file in the location that it expected the server to
manipulate though. We'd have to push that for the test to be legit.
We could write a
commit_and_pushfunction for our tests, but thatfunction will be a pain in the ass. Instead we can just go though a full
crate upload twice to test what we want.