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.

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

Changes for fixing golangci-lint issues #87

Merged
merged 7 commits into from
Jan 29, 2021

Conversation

sivachandran
Copy link
Contributor

Addresses issue #83

Change summary

  • Did go mod tidy to remove unneeded module references
  • Added godoc comment to exported types and functions
  • Made minor changes in the source as per the linter advice

There are many more linter issues that require authors' consensus on how to resolve them.

Refer linter-issues.txt for pending linter issues.

@sivachandran sivachandran changed the title [Changes for fixing golangci-lint issues Changes for fixing golangci-lint issues Jan 21, 2021
@priyaaank priyaaank merged commit 0e04c10 into clamp-orchestrator:master Jan 29, 2021
@priyaaank priyaaank added this to the Release Thor milestone Jan 29, 2021
@priyaaank
Copy link
Member

Tests need to be fixed for this to be merged successfullyy

@sivachandran
Copy link
Contributor Author

@priyaaank There are many test failures even in the master branch. Do you want to fix master test failures first?

@priyaaank
Copy link
Member

Hey, @sivachandran Thanks for the feedback.

The CI tests were green here https://app.circleci.com/pipelines/github/clamp-orchestrator/clamp-core?branch=master

However, I checked and looks like that while the tests are passing there is a step that uploads the code quality report to CodeClimate with every test run. This is intended to execute only on the CI, but executes locally as well and fails. Although I doubt this is the error you encountered. But it has been fixed as part of PR #90

The Latest CI execution is here https://app.circleci.com/pipelines/github/clamp-orchestrator/clamp-core/120/workflows/f11a3bf7-9011-4860-a273-e7519eb57c3a/jobs/146
requires login with your GitHub id

Since there are various kinds of tests (unit, integration, and functional) and require several moving parts to be set up (Kafka, Zookeeper, rabbitmq, Postgres, etc.) And if one of those components is not working, the tests might fail. I think this is what you may have encountered.

Here are the remediation steps for that. I am also updating documentation here

At the end of the test, you should see that artifact could not be saved. This is expected since CircleCI CLI does not support it in the local environment. However, the tests should be green with all components managed by the CLI. This is how output would look like
Screen Shot 2021-01-30 at 12 19 25 PM

You can choose to avoid circleci CLI and run the test as specified in the documentation here https://clamp-orchestrator.github.io/clamp-orchestrator/docs/core-contribution-guide#running-tests-locally
But in that case please ensure all components are set up and running as expected.

This is how the outcome of the build should look
Screen Shot 2021-01-30 at 12 08 25 PM

Let me know if this helps with the setup and helps you run the tests successfully.

@sivachandran
Copy link
Contributor Author

Thank you @priyaaank for the detailed response.

I assumed the unit tests do not require external dependencies like RabbitMQ or Kafka. Now, relooking at the error log, I can see many errors are related to not having the dependencies running.

I will set up CircleCI CLI and try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants