Tests

Add some tests to this repository:

• hadolint for Dockerfiles
• shellcheck for bash and sh scripts
• yamllint for YAML files

Currently marking every job with allow_failure: yes, but if this is accepted, I guess the next step would be to fix the "problems" and remove those lines.

@jon.azpiazu you need to enable the runner for this repo yourself, since I don't have the rights to do so.

@jon.azpiazu you need to enable the runner for this repo yourself, since I don't have the rights to do so.

Done now.

You can see the tests ran (and failed).

What do you think of this? Do you think this may be helpful? Would you be in favor of merging and either fixing the errors or relaxing the test severity in future MRs, or would you rather merge when the tests are green?

Some of the errors are clearly overkill in my opinion. For example:

deploy/base/Dockerfile:5 DL3008 Pin versions in apt get install. Instead of `apt-get install <package>` use `apt-get install <package>=<version>`

This is probably something we do not want to do.

So the question is: either relax the tests specifying rules we do not want to comply with (and fix what we think is reasonable), leave as it is but ignore (not very useful), or completely remove the tests.

added 4 commits

• a18e72d0 - Relax Docker test severity and fix remaining issues
• dd5e4065 - Run yamllint with 'relaxed' settings
• 828e137f - Relax some shellcheck tests and fix remaining issues
• 0cba1e96 - Require Dockerfile and shellcheck jobs to succeed

Compare with previous version

Some of the errors are clearly overkill in my opinion.

I agree.

I went ahead and disabled some of the tests that seemed too unreasonable, and fixed the remaining errors. In summary:

• For Dockerfiles, ignore the requirement to pin versions installed though apt-get and the requirement to use tagged images in the FROM statement. The latter isn't really important in one of our images since it's a temporary image that is generated in the same job and later discarded, and in the other error we are actually using a tagged image but it probably isn't recognized because an ARG is used.
• For YAML files, switched to using the relaxed mode. There are still a bunch of warnings, but those shouldn't make the job fail. We could add a custom config to suppress the warnings, but I'm not sure it's worth it. However, there are two other errors due to duplicate keys in the custom rosdep that are actually important and had gone unnoticed. We should either fix them here or open a new issue to track them.
• Fixed most of the shellcheck problems and suppressed a couple of them.

mentioned in issue #10 (closed)

I suggest we add the line-length rule to be ignored in the YAML test, and then merge this.

added 1 commit

• d6fed684 - Disable line-length warning by extending 'relaxed' yamllint configuration

Compare with previous version

I suggest we add the line-length rule to be ignored in the YAML test, and then merge this.

Done. The test still fails because of #10 (closed), but we no longer see the line length-related warnings.

mentioned in merge request !26 (merged)

added 2 commits

• b32cca0c - Delete duplicate keys
• 053d5530 - Merge branch '10-duplicated-keys-in-rosdep-database' into 'tests'

Compare with previous version

added 1 commit

• f2c5f4e4 - Remove allow failure for tests

Compare with previous version

merged

mentioned in commit 1494883b

mentioned in merge request !24 (merged)