codeclimate / ruby-test-reporter

DEPRECATED Uploads Ruby test coverage data to Code Climate
https://codeclimate.com
Other
92 stars 92 forks source link

command not found: git #171

Closed lededje closed 7 years ago

lededje commented 7 years ago

Building alpine images to lower docker image sizes means git is not available. Is there a way to run this command without git?

chazsconi commented 7 years ago

Similarly it would be nice to be able to run the command and optionally passing in the the git information (revision, branch etc) as an argument rather having it use git to fetch the information. In a docker environment, it would be quite common to not have the .git folder inside the image as it makes the image size much bigger.

dblandin commented 7 years ago

Hi @lededje,

Do any of the currently supported CI mappings address this issue?

https://github.com/codeclimate/ruby-test-reporter/blob/master/lib/code_climate/test_reporter/ci.rb

As of #177, having git and the git repository available should no longer be a hard requirement.

jakehow commented 7 years ago

Hey @dblandin

You guys currently have committed_at as a required payload value, but only the CodeShip branch of your conditional you linked to provides the ability to set that value.

When I set the CIRCLE env vars I get the following error:

CodeClimate::TestReporter::InvalidPayload: A git commit timestamp was not found in the test report payload

I think either making a REQUIRED_PAYLOAD_ATTRS array, and adding that to your spec suite to confirm you have implemented it in every CI you handle, or letting us just set the required env vars directly w/o regard to which service we are on would be usable resolutions. I don't see a way to add to the payload via env var in the current codebase.

One challenge with trying to make an adapter for every CI is that some don't provide all the required vars by default. CircleCI, where we are, does not provide COMMITTED_AT, this is easy to fix, but is on the developer probably.

Since I presume this issue mostly comes up in dockerized CI envs, where we need to send the vars to docker anyway, generic env vars seem ok to me.

I am getting around this right now by pretending we are running on CodeShip and setting all their expected vars. This has one downside which is that the name attribute on the payload is hardcoded, so CC will actually think we are using CodeShip, not sure if that matters.

Command I use for others finding this issue:

docker exec \
  -e CI_NAME=codeship \
  -e CI_BUILD_ID=$CIRCLE_BUILD_NUM \
  -e CI_BRANCH=$CIRCLE_BRANCH \
  -e CI_COMMIT_ID=$CIRCLE_SHA1 \
  -e CI_BUILD_URL=$CIRCLE_BUILD_URL \
  -e CI_TIMESTAMP=$(git log -1 --pretty=format:%ct) \
  -it <my-container> \
  bundle exec codeclimate-test-reporter

You can see there that I am manually setting the timestamp using the same command this gem does internally.

dblandin commented 7 years ago

@jakehow This sounds like a different issue than the one reported here. We're also using CircleCI, running our test suite within a docker container, and we mount the git directory into the container so that the test reporter has access to the repository:

docker run \
  --env CIRCLECI \
  --env CIRCLE_BUILD_NUM \
  --env CIRCLE_BRANCH \
  --env CIRCLE_SHA1 \
  --env CODECLIMATE_REPO_TOKEN \
  --volume "$(pwd)/.git:/usr/src/app/.git" \
  your-image-name \
  /bin/sh -c "bundle exec rspec && bundle exec codeclimate-test-reporter"

We highly recommend giving the test reporter process access to a git directory when possible so that we can fetch reliable blob IDs from git and reliably match your test report data to source code on codeclimate.com.

We have a fallback manual calculation in some reporters but it's less reliable and could prevent test coverage from linking up properly.

jakehow commented 7 years ago

@dblandin I don't think this is a different issue. If the container doesn't have the git binary on it, then mounting the directory as a volume doesn't help.

dblandin commented 7 years ago

@jakehow Ahh, gotcha. Yes, in that case this wouldn't work. Do you have git installed on the host machine?

Another option is volume mounting the ./coverage directory, installing the codeclimate-test-reporter on the host, and invoking it after the test suite runs in the container.

That might look like:

test:
  override:
     - >
       docker run
       --volume $(pwd)/coverage:/usr/src/app/coverage
       your-image-name bundle exec rspec
  post:
    - gem install codeclimate-test-reporter && codeclimate-test-reporter
jakehow commented 7 years ago

@dblandin we are using Circle 2.0 where the docker host is remote, so mounting volumes doesn't work for much since the machine you are running the command on is not where the volume is created.

However, I still don't think this would fix the issue. committed_at is only set if the git binary is available or if the value is in the payload from the CI reporter.

You can see the code where this value is set here -> https://github.com/codeclimate/ruby-test-reporter/blob/4bfe8f69d6cbd777dd8ceba5b2cbc9c3e764721d/lib/code_climate/test_reporter/git.rb#L21-L23

There is no other way to set it in the CI reporter other than pretending to be CodeShip.

You can see where it is set in the CodeShip branch of the conditional in Ci.service_data here -> https://github.com/codeclimate/ruby-test-reporter/blob/4bfe8f69d6cbd777dd8ceba5b2cbc9c3e764721d/lib/code_climate/test_reporter/ci.rb#L74

You can see where your PayloadValidator requires that it be set here -> https://github.com/codeclimate/ruby-test-reporter/blob/925d5b2fd9e64964ac6f92685642018f29cf72f4/lib/code_climate/test_reporter/payload_validator.rb#L16

The issue I pointed out is a legitimate bug (and an easily fixable one) in the library if the intended behavior is to be able to run without git, which, like I mentioned, we are successfully doing by pretending to send from CodeShip.

dblandin commented 7 years ago

Hey @jakehow,

if the intended behavior is to be able to run without git

Running the test reporter without git and a git repository present is not a feature of our reporters overall. We've received community contributions from users who wish to pipe this data in from certain CI environments, like CodeShip. See https://github.com/codeclimate/ruby-test-reporter/pull/168 and https://github.com/codeclimate/ruby-test-reporter/issues/156.

We'd be happy to review a PR for either CLI arguments or a custom CI override mapping, but will not be prioritizing new work for this reporter internally.

We're working on a new test reporter supporting multiple coverage report formats and parallelized reporting. Any support for test coverage reporting without git and a git repository present would be discussed and implemented in the new reporter. I've just opened up https://github.com/codeclimate/test-reporter/issues/94 with some additional context and I hope to continue the discussion there.

Supporting test coverage reporting without a git directory is possible but might be implemented with a warning since information (particularly the file blob id) fetched via alternate methods will be less reliable and may not link properly with source code on codeclimate.com.

Thanks for your feedback.

dblandin commented 7 years ago

Hey @jakehow! Thanks again for the issue!

I'd still encourage you to check out our new test reporter. I'm going to go ahead and close this issue as we're winding down development of this ruby-specific test reporter.