integration tests for our Dockerfile and ruleset #94
Conversation
phracek
force-pushed
the
set_images_for_tests
branch
6 times, most recently
from
118b460
to
18182f7
Compare
phracek
requested review from
jpopelka,
kosciCZ and
lachmanfrantisek
and removed request for
kosciCZ
Makefile
Outdated
| clean: | ||
| git clean -dfx | ||
|
|
||
| install: clean |
There was a problem hiding this comment.
install requires clean and clean removes all untracked files, that's pretty bold
Makefile
Outdated
|
|
||
| build-test-container: | ||
| docker build --network host --tag=$(TEST_IMAGE_NAME) -f ./Dockerfile.tests . | ||
|
|
||
| build-labels-container: |
There was a problem hiding this comment.
Just a detail, but, you don't build container, you build image (or container image), so I'd rename:
- build-test-container -> build-test-image
- build-labels-container -> build-labels-image
tests/data/Dockerfile
Outdated
|
|
||
| LABEL maintainer="Petr Hracek <phracek@redhat.com>" | ||
|
|
||
| LABEL name="${NAME}" \ |
There was a problem hiding this comment.
Is that some practice to do these in two separate steps ? Or can they be merged into one ?
|
|
||
| with open(temp_file_name, 'w') as outfile: | ||
| json.dump(colin_json, outfile, ensure_ascii=True) | ||
| return temp_file_name |
There was a problem hiding this comment.
Not sure I understand.
So you create temp file, store its name and delete it.
Then open it again, dump a json inside, close and return its name.
Why do you delete and open it again ?
Why the dir='/tmp' - isn't it default?
What about:
tmpfile = tempfile.NamedTemporaryFile(mode='w', delete=False)
json.dump(colin_json, tmpfile, ensure_ascii=True)
tmpfile.close()
return tmpfile.nameThere was a problem hiding this comment.
I wanted to create a random name for later on usage.
tests/integration/test_labels.py
Outdated
| import colin | ||
|
|
||
| def get_colin_image(): | ||
| return colin.run("colin-labels", ruleset_name="fedora") |
There was a problem hiding this comment.
From the function name I'd expect it'd get me some image, while it runs checks, doesn't it ?
The same applies to test_ruleset_file.py:get_colin_labels_image()
| } | ||
|
|
||
|
|
||
| with open(generate_json_file(colin_json), "r") as f: |
There was a problem hiding this comment.
this is pretty stupid, colin should be able to accept that directly
There was a problem hiding this comment.
Shall I wait for solving #98 ? WDYT?
There was a problem hiding this comment.
no, leave it as it is, we'll fix it later
phracek
force-pushed
the
set_images_for_tests
branch
3 times, most recently
from
d45d2d0
to
34317bb
Compare
|
I have added also tests for cli, especially Click. |
|
|
||
| Error: Missing argument "target". | ||
| """ | ||
| assert result.exit_code == 2 |
There was a problem hiding this comment.
Hmm, this is the same exit code as on fail. We should change that. (in the code)
Makefile
Outdated
| build-labels-image: | ||
| cd tests/data && docker build --tag=$(TEST_IMAGE_LABELS_NAME) . | ||
|
|
||
| test-in-container: build-test-image |
There was a problem hiding this comment.
Also build-test-image has to be added.
There was a problem hiding this comment.
you mean build-labels-image, right?
tests/unit/test_cli.py
Outdated
|
|
||
| def test_list_checks(): | ||
| result = _call_colin(list_checks) | ||
| expected_output = """LABELS: |
There was a problem hiding this comment.
The output is something, that can be easily changed a bit in the future, but for now, it's ok.
tests/unit/test_cli.py
Outdated
| from colin.cli.colin import check, list_checks, list_rulesets | ||
|
|
||
| def _call_colin(fnc): | ||
| runner = CliRunner() |
There was a problem hiding this comment.
I forgot we can test click like this -- we can make more checks like this as a next step. (e.g. version would be good)
|
@phracek Is this all for now? I think we can merge that and add more tests later. |
Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
phracek
force-pushed
the
set_images_for_tests
branch
from
0cc54c9
to
122da4c
Compare
Signed-off-by: Petr "Stone" Hracek phracek@redhat.com
This Pull Request brings a Dockerfile with defined set of labels and two integration tests
for rulesets and labels.
It solves particularly #63 and #56.
If you want some other Dockerfiles, just let me know and I will add it.