@athoscr Opened a new one, from a different branch without duplicated commits

Hi Gabriel,

This looks good :) Thank you for all the effort!

I still have a few requests and questions before we merge this:

1 - We do not need commit 40b6003 anymore, since this file was never used at all, right? To make the PR cleaner, you can rebase the branch with -i option and squash 40b6003 and 01d5e91, since the file in question was never needed at all.

2 - Is there any reason for downloading docker RPM from upstream instead of using the fedora package (not sure if I already asked you this question before...)? If not, using the Fedora package seems to be a better idea: we would not need to keep updating the package version every 6 months in the make file. In special, note that Fedora 24 already reached its End of Life.

3 - The name of the targets in the makefile could improve. Maybe make docker-env and make run-docker-env would be more meaningful names for those targets.

@athoscr All three done :)

I believe all the changes are ready to be merged! Thank you for the effort, Gabriel :)

Would you mind squashing it all into a single commit to make this cleaner and (always) functional?

Yesterday I found out a new issue with the docker environment but already solved it. Today I'll commit it and sqash all commits into one

Hi Gabriel, thanks for the effort, the PR looks quite clean now :)

Have you tried running the test suite with this setup? Are all tests passing for you?

Since you guys are using MacOSes for development and I have no intentions to use a docker container for development purposes, I do not want to block this PR any longer. Note that I got
kiskadee.tests.test_plugins.TestDebianFetcher failing and kiskadee.tests.test_runner.TestAnalyzers hanging.

I did not try to debug the problem though. Any comments?

Note that I did change the net option to host here and I can access the container from the host and the host from the container with your setup.

This is not a blocker, but it would be nice to install the dependencies directly in the container root instead of using a virtualenv inside it (it seems pointless and has too much footprint in the working directory).

I took a look at the test failing and it is the following:

def test_compare_gt_version(self):
        new = '1.1.1'
        old = '1.1.0'
        result = self.debian_fetcher.compare_versions(new, old)
        self.assertTrue(result)

Also took a look at the calling function but I'm still not quite sure what this was supposed to do and how.

This is the hanging test:

def test_generate_a_firehose_report(self):
        source_to_analysis = {
                'name': 'test',
                'version': '1.0.0',
                'fetcher': kiskadee.fetchers.example.Fetcher()
        }
        self.runner.call_analyzers(source_to_analysis)
        analyzed_pkg = self.runner.kiskadee_queue.dequeue_result()
        self.assertEqual(analyzed_pkg['name'], source_to_analysis['name'])
        self.assertIn('cppcheck', analyzed_pkg['results'])
        # TODO: fix issue #44
        # self.assertIn('flawfinder', analyzed_pkg['results'])

And when that test is not hanging, this also fails:

def test_run_analyzer(self):
        source_to_analysis = {
                'name': 'test',
                'version': '1.0.0',
                'fetcher': kiskadee.fetchers.example.Fetcher()
        }
        source_path = self.runner._path_to_uncompressed_source(
                source_to_analysis, kiskadee.fetchers.example.Fetcher()
            )
        firehose_report = self.runner.analyze(
                self.deb_pkg, "cppcheck", source_path)
        self.assertIsNotNone(firehose_report)

As the first failing test, I'm not exactly sure what is causing this trouble and how to solve it.

For the -net=host: this is no longer necessary because, as I say in README, just change the host name to solve the initial problem.

For the env issue, I tried creating a docker image installing python dependencies directly on docker without creating a python environment and it failed. I talked about that on IRCC kiskadee channel.

The first test uses dpk to compare package versions. You must make sure your container can run dpk. It would also be nice to find a better way to run that comparison without calling dpk (but that is another issue.

I did not understand your point on the second failing test, are you proposing something?

As I said, as soon as tests are passing in the docker environment, we can accept the PR! I would not oppose to have changes in the test suite if they are needed.

So the docker must have dpk installed? If I try to run it inside docker I get bash: dpk: command not found

My point in the other tests is the same on the first one: I don't understand them enough so I can try debugging why they are crashing.

I agree the PR shouldn't be merged until all the tests are passing.

It the program name is dpkg. Did you solve this one?

Any news here?

Closing this PR. We will no longer merge this.