Skip to content

Conversation

RamiSouai
Copy link
Member

@RamiSouai RamiSouai commented Mar 9, 2022

Description

The integration-test command for hooks is now the same for all hooks, and uses npm run tests (With the exception of Persistance-DefectDojo which is in Java).

Now each hook can decide for itself what the npm run test command does in it's package.json file. For example, notification and cascading-scans hooks, which are in Typescript, add an extra build step.

Checklist

  • Test your changes as thoroughly as possible before you commit them. Preferably, automate your test by unit/integration tests.
  • Make sure npm test runs for the whole project.

Signed-off-by: Rami Souai <rami.souai@iteratec.com>
Signed-off-by: Rami Souai <rami.souai@iteratec.com>
@RamiSouai RamiSouai added bug Bugs enhancement New feature or request testing Improvements or additions regarding the test setup hook Implement or update a hook labels Mar 9, 2022
Signed-off-by: Simon Hülkenberg <simon.huelkenberg@iteratec.com>
@RamiSouai RamiSouai changed the title Maintenance/generic makefile hooks Inherit Makefile Integration Tests command for hooks Mar 15, 2022
@RamiSouai RamiSouai marked this pull request as ready for review March 15, 2022 17:44
@Ilyesbdlala Ilyesbdlala added planned Issues we will do in the next sprint. and removed hook Implement or update a hook enhancement New feature or request bug Bugs labels Mar 16, 2022
Copy link
Member

@Weltraumschaf Weltraumschaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Only .PHONY tags we should add to at least all touched Makefiles.

As requested in the review

Signed-off-by: Ilyes Ben Dlala <ilyes.bendlala@iteratec.com>
This is done to include the semgrep integration test fix
…ook folder

This is done so these tests would run when "make integration tests" is called
in the cascading-scan hook folder
This goes hand in hand with issue #890

Signed-off-by: Ilyes Ben Dlala <ilyes.bendlala@iteratec.com>
@J12934 J12934 requested a review from Weltraumschaf March 16, 2022 16:34
include ../../hooks.mk

.PHONY: test-2
test-2: | clean-integration-tests unit-tests docker-build docker-export kind-import deploy deploy-test-deps-2 integration-tests-2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the | here? Never seen this before and I'm too stupid to google it...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second question: What does the 2 mean in the target name? We should avoid magic numbers in names.

@rseedorff rseedorff merged commit 5375eeb into main Mar 21, 2022
@rseedorff rseedorff deleted the maintenance/generic-makefile-hooks branch March 21, 2022 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
planned Issues we will do in the next sprint. testing Improvements or additions regarding the test setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants