Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Remove codeclimate init #78

Closed
wants to merge 2 commits into from
Closed

Remove codeclimate init #78

wants to merge 2 commits into from

Conversation

maxjacobson
Copy link
Contributor

@maxjacobson maxjacobson commented Dec 11, 2017

This subcommand was removed. We have an open issue about it on the repo,
and the tests are currently timing out on master, which is blocking an
unrelated fix. I'm hopeful this will get the tests green.

Related to #75

This subcommand was removed. We have an open issue about it on the repo,
and the tests are currently timing out on master, which is blocking an
unrelated fix. I'm hopeful this will get the tests green.
@maxjacobson
Copy link
Contributor Author

Sigh. That didn't help make it green. I'm not sure how to debug this 😞. If i could run the tests locally I think I'd be able to, but I'm not sure how to do that. Any thoughts @Arcanemagus ?

@Arcanemagus
Copy link
Member

Assuming you can run codeclimate locally you should just be able to do apm test from the place where you have the code checked out. I'm on Windows so I can't debug this locally myself...

@cgalvarez
Copy link
Contributor

I faced this exact problem previously. @pointlessone had to increase the timeout for the promises in the package tests on the CI environment to 60 seconds here with this PR to get the tests passing in CI.

The problem right now is that the CodeClimate cli takes much more time now to finish. I'm having times of ~150s locally right now, so maybe in CI it takes even more.

linter-codeclimate: `.mdlrc` analysis took: 131433.78
linter-codeclimate: `.codeclimate.yml` analysis took: 134356.05000000002
linter-codeclimate: `package.json` analysis took: 117612.29500000001
linter-codeclimate: `README.md` analysis took: 141472.95
linter-codeclimate: `TODO.md` analysis took: 145353.2

@maxjacobson Like @Arcanemagus says, you only need to run your-local-fork$ apm test.

I've checked that increasing that timeout to 150 seconds (locally), the following error appears:

The codeclimate provider for Linter
  it works with a valid .codeclimate.yml file
    Expected promise to be resolved, but it was rejected with: Cannot convert undefined or null to object {  }

As this is the same error that #77 reports, after applying its fix locally and rerunning the tests, we get:

The codeclimate provider for Linter
  it works with a valid .codeclimate.yml file
    Expected 'RUBOCOP: Do not put a space between a method name and the opening parenthesis. [Rubocop/Layout/SpaceAfterMethodName]' to be 'RUBOCOP: Unused method argument - `bar`. If it's necessary, use `_` or `_bar` as an argument name to indicate that it won't be used. You can also write as `foo(*)` if you want the method to accept any arguments but don't care about them. [Rubocop/Lint/UnusedMethodArgument]'.
      Error: Expected 'RUBOCOP: Do not put a space between a method name and the opening parenthesis. [Rubocop/Layout/SpaceAfterMethodName]' to be 'RUBOCOP: Unused method argument - `bar`. If it's necessary, use `_` or `_bar` as an argument name to indicate that it won't be used. You can also write as `foo(*)` if you want the method to accept any arguments but don't care about them. [Rubocop/Lint/UnusedMethodArgument]'.
      at /opt/cgalvarez/contributing/linter-codeclimate/spec/linter-codeclimate-spec.js:27:39
    Expected [ [ 1, 9 ], [ 1, 10 ] ] to equal [ [ 1, 11 ], [ 1, 14 ] ].
      Error: Expected [ [ 1, 9 ], [ 1, 10 ] ] to equal [ [ 1, 11 ], [ 1, 14 ] ].
      at /opt/cgalvarez/contributing/linter-codeclimate/spec/linter-codeclimate-spec.js:37:49

Finished in 21.854 seconds
1 test, 8 assertions, 2 failures, 0 skipped

It seems that after fixing code with #77 the time the cli takes to finish decreases A LOT, again to 20-30s.

The new error is due to new issues reported by the cli. The expected first issue is not such, so we need to change the .codeclimate.yml of the test and include a new .rubocop.yml to fix that test as follows:

# spec/fixtures/.rubocop.yml
Layout/SpaceAfterMethodName:
  Enabled: false

Layout/SpaceAroundEqualsInParameterDefault:
  Enabled: false

Layout/SpaceBeforeBlockBraces:
  Enabled: false

Style/Lambda:
  Enabled: false
# spec/fixtures/.codeclimate.yml
---
engines:
  rubocop:
    enabled: true
  fixme:
    enabled: true
  structure:
    enabled: false
ratings:
  paths:
  - "**.coffee"
exclude_paths:
- node_modules/**/*

Making these changes, the tests pass locally, and so should happen in CI.

@cgalvarez
Copy link
Contributor

I've created PR #82 which achieves the same as this one, but the CI builds successfully.

@cgalvarez cgalvarez mentioned this pull request Feb 25, 2018
6 tasks
@Arcanemagus Arcanemagus deleted the mj/remove-init branch April 21, 2018 05:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants