Skip to content

Inherit Makefile Integration Tests command for hooks #1028

Merged
rfelber merged 8 commits intomainfrom
maintenance/generic-makefile-hooks
Mar 21, 2022
Merged

Inherit Makefile Integration Tests command for hooks #1028
rfelber merged 8 commits intomainfrom
maintenance/generic-makefile-hooks

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.

@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
@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 <[email protected]>
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 <[email protected]>
@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.

@rfelber rfelber merged commit 5375eeb into main Mar 21, 2022
@rfelber rfelber 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