Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI | Fix config retrieval logic (AST-76622) #957

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AlvoBen
Copy link
Collaborator

@AlvoBen AlvoBen commented Dec 5, 2024

Introduce a new integration test to verify handling of the resubmit flag during scan creation for non-existent projects, ensuring successful creation with default config. Additionally, fix the logic to retrieve configuration only when scans are available, preventing potential nil pointer dereferences.

By submitting a PR to this repository, you agree to the terms within the Checkmarx Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.

Description

Introduce a new integration test to verify handling of the resubmit flag during scan creation for non-existent projects, ensuring successful creation with default config. Additionally, fix the logic to retrieve configuration only when scans are available, preventing potential nil pointer dereferences.

References

https://checkmarx.atlassian.net/browse/AST-76622

Testing

Added integration test

Checklist

  • I have added documentation for new/changed functionality in this PR (if applicable).
  • I have updated the CLI help for new/changed functionality in this PR (if applicable).
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used

Introduce a new integration test to verify handling of the resubmit flag during scan creation for non-existent projects, ensuring successful creation with default config. Additionally, fix the logic to retrieve configuration only when scans are available, preventing potential nil pointer dereferences.
@github-actions github-actions bot added the bug Something isn't working label Dec 5, 2024
@AlvoBen AlvoBen changed the title Add test for resubmit flag and fix config retrieval logic Add test for resubmit flag and fix config retrieval logic (AST-76622) Dec 5, 2024
@AlvoBen AlvoBen changed the title Add test for resubmit flag and fix config retrieval logic (AST-76622) CLI | Fix config retrieval logic (AST-76622) Dec 5, 2024
Copy link

github-actions bot commented Dec 5, 2024

Logo
Checkmarx One – Scan Summary & Detailsc6437900-6c8c-42ef-8ad5-15177cccf528

No New Or Fixed Issues Found

actualScanTypes = strings.Join(engines, ",")

if len(allScansModel.Scans) > 0 {
config = allScansModel.Scans[0].Metadata.Configs
Copy link

Choose a reason for hiding this comment

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

allScansModel.Scans[0] -> you can extract the value once and then use it.

@@ -1972,3 +1972,17 @@ func TestCreateAsyncScan_CallExportServiceBeforeScanFinishWithRetry_Success(t *t
asserts.Nil(t, err)
assert.Assert(t, exportRes != nil, "Export response should not be nil")
}

func TestCreateScanWithResubmitFlag_ProjectNotExist_ScanCreatedSuccessfullyWithDefaultConfig(t *testing.T) {
Copy link

Choose a reason for hiding this comment

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

can we some how assert the config to check it is the default configs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can probably create unit test for this function that will check that the config that got returned is empty

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added unit test

Simplified the retrieval of config and engines by using a single variable, `scanModelResponse`, to improve code readability and maintainability. This change consolidates access to elements of `allScansModel.Scans[0]` into one clear reference point, making the code easier to understand and modify in the future.
Updated the ScansMockWrapper's Get method to return an empty configuration if the project ID is "non-existent-project". Added a test to ensure that the method behaves correctly in this scenario, verifying that an empty configuration is returned when attempting to resubmit a scan for a non-existent project.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants