Fail with error when actual report does not exist (!64) · Merge requests · GitLab.org / security-products / analyzers / integration-test

Adam Cohen requested to merge fail-when-report-is-nil into main Sep 11, 2024

What does this MR do?

Changes the behaviour of the integration test to fail if a report is missing. The old behaviour would mark the test as pending and the test would succeed.
Outputs the expected path to the missing report when it doesn't exist.
Filters out all but two lines from the rspec failed example backtrace output, in order to reduce verbosity.

Further Details

In Skip shared examples and debug scan when report... (!12 - merged) • Fabien Catteau • 14.7, the behaviour was changed so that if the analyzer being tested does not produce a report, the corresponding shared examples will return a pending status and output a message explaining that the report is missing, however, the test still succeeds.

The change in Skip shared examples and debug scan when report... (!12 - merged) • Fabien Catteau • 14.7 was made because the output of rspec in this situation is extremely verbose, as explained here, and will provide an unhelpful error message: undefined method [] for nil:NilClass along with a full stack trace:

Click to expand full error message

  7) running image with test project with java when using maven build-tool on Java 11 created report behaves like recorded report remediations is equivalent
     Failure/Error: subject { GitlabSecure::IntegrationTest::Comparable.remediations(report["remediations"]) }

     NoMethodError:
       undefined method `[]' for nil:NilClass
     Shared Example Group: "recorded report" called from ./spec/semgrep_image_spec.rb:268
     # /usr/lib/ruby/gems/3.2.0/gems/gitlab_secure-integration_test-0.1.0/lib/gitlab_secure/integration_test/shared_examples/report_shared_examples.rb:93:in `block (3 levels) in <top (required)>'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/memoized_helpers.rb:317:in `block (2 levels) in let'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/memoized_helpers.rb:157:in `block (3 levels) in fetch_or_store'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/memoized_helpers.rb:157:in `fetch'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/memoized_helpers.rb:157:in `block (2 levels) in fetch_or_store'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-support-3.10.3/lib/rspec/support/reentrant_mutex.rb:23:in `synchronize'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/memoized_helpers.rb:156:in `block in fetch_or_store'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/memoized_helpers.rb:155:in `fetch'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/memoized_helpers.rb:155:in `fetch_or_store'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/memoized_helpers.rb:317:in `block in let'
     # /usr/lib/ruby/gems/3.2.0/gems/gitlab_secure-integration_test-0.1.0/lib/gitlab_secure/integration_test/shared_examples/report_shared_examples.rb:98:in `block (3 levels) in <top (required)>'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example.rb:262:in `instance_exec'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example.rb:262:in `block in run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example.rb:508:in `block in with_around_and_singleton_context_hooks'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example.rb:465:in `block in with_around_example_hooks'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/hooks.rb:486:in `block in run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/hooks.rb:624:in `run_around_example_hooks_for'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/hooks.rb:486:in `run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example.rb:465:in `with_around_example_hooks'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example.rb:508:in `with_around_and_singleton_context_hooks'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example.rb:259:in `run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:644:in `block in run_examples'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:640:in `map'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:640:in `run_examples'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:606:in `run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `block in run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `map'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `block in run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `map'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `block in run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `map'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `block in run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `map'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `block in run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `map'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `block in run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `map'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/example_group.rb:607:in `run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/runner.rb:121:in `block (3 levels) in run_specs'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/runner.rb:121:in `map'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/runner.rb:121:in `block (2 levels) in run_specs'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/configuration.rb:2067:in `with_suite_hooks'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/runner.rb:116:in `block in run_specs'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/reporter.rb:74:in `report'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/runner.rb:115:in `run_specs'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/runner.rb:89:in `run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/runner.rb:71:in `run'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/lib/rspec/core/runner.rb:45:in `invoke'
     # /usr/lib/ruby/gems/3.2.0/gems/rspec-core-3.10.2/exe/rspec:4:in `<top (required)>'
     # /usr/bin/rspec:25:in `load'
     # /usr/bin/rspec:25:in `<main>'
     #
     #   Showing full backtrace because every line was filtered out.
     #   See docs for RSpec::Configuration#backtrace_exclusion_patterns and
     #   RSpec::Configuration#backtrace_inclusion_patterns for more information.

However, suppressing failures due to missing report files is dangerous because it can hide incorrect code and bugs.

For example, I recently created an MR Remove unused global vars from integration test (semgrep!488 - merged) • Adam Cohen • 17.4 which introduced a bug and should have caused a large number of tests to fail:

Pending: (Failures listed here are expected and do not affect your suite's status)
  1) running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report version 
     # report is missing
  2) running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report scan 
     # report is missing
  3) running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report vulnerabilities 
     # report is missing
<snip>
  32) running image with test project with java when using maven build-tool for multimodules created report behaves like valid report passes schema validation without errors
     # report is missing
Finished in 8 minutes 49 seconds (files took 0.34546 seconds to load)
427 examples, 0 failures, 32 pending

However, the behaviour implemented in Skip shared examples and debug scan when report... (!12 - merged) • Fabien Catteau • 14.7 caused these rspec examples to be marked as pending rather than failures. Because the job succeeded, we weren't aware of these test failures and merged the MR, which substantially reduced our test coverage, and put us at risk of introducing new bugs.

In order to prevent these types of situations from occurring in the future, I've created this MR to cause the integration test job to fail if an expected report does not exist.

What are the relevant issue numbers?

N/A

Testing

Before the change in this MR:

rspec examples with a missing report are listed as pending and the test succeeds:

Pending: (Failures listed here are expected and do not affect your suite's status)
  1) running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report version 
     # report is missing
     # 
  2) running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report scan 
     # report is missing
     # 
  3) running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report vulnerabilities 
     # report is missing
     # 
  4) running image with test project with java when using maven build-tool on Java 11 created report behaves like recorded report scan is equivalent
     # report is missing
     # 

<snip>

Finished in 8 minutes 49 seconds (files took 0.34546 seconds to load)
427 examples, 0 failures, 32 pending

After the change in this MR:

rspec examples with a missing report cause the test to fail:

Failures:
  1) running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report version is expected to exist "/builds/gitlab-org/security-products/analyzers/semgrep/tmp/test-22721/java/maven/running-image-with-...aven-build-tool-on-java-11-created-report-behaves-like-non-empty-report-version/gl-sast-report.json"
     Failure/Error: DEFAULT_FAILURE_NOTIFIER = lambda { |failure, _opts| raise failure }
       Expected report '/builds/gitlab-org/security-products/analyzers/semgrep/tmp/test-22721/java/maven/running-image-with-test-project-with-java-when-using-maven-build-tool-on-java-11-created-report-behaves-like-non-empty-report-version/gl-sast-report.json' to exist
     Shared Example Group: "non-empty report" called from ./spec/semgrep_image_spec.rb:269
     # /usr/bin/rspec:25:in `load'
     # /usr/bin/rspec:25:in `<main>'
  2) running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report scan is expected to exist "/builds/gitlab-org/security-products/analyzers/semgrep/tmp/test-22721/java/maven/running-image-with-...aven-build-tool-on-java-11-created-report-behaves-like-non-empty-report-version/gl-sast-report.json"
     Failure/Error: DEFAULT_FAILURE_NOTIFIER = lambda { |failure, _opts| raise failure }
       Expected report '/builds/gitlab-org/security-products/analyzers/semgrep/tmp/test-22721/java/maven/running-image-with-test-project-with-java-when-using-maven-build-tool-on-java-11-created-report-behaves-like-non-empty-report-version/gl-sast-report.json' to exist
     Shared Example Group: "non-empty report" called from ./spec/semgrep_image_spec.rb:269
     # /usr/bin/rspec:25:in `load'
     # /usr/bin/rspec:25:in `<main>'
  3) running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report vulnerabilities is expected to exist "/builds/gitlab-org/security-products/analyzers/semgrep/tmp/test-22721/java/maven/running-image-with-...aven-build-tool-on-java-11-created-report-behaves-like-non-empty-report-version/gl-sast-report.json"
     Failure/Error: DEFAULT_FAILURE_NOTIFIER = lambda { |failure, _opts| raise failure }
       Expected report '/builds/gitlab-org/security-products/analyzers/semgrep/tmp/test-22721/java/maven/running-image-with-test-project-with-java-when-using-maven-build-tool-on-java-11-created-report-behaves-like-non-empty-report-version/gl-sast-report.json' to exist
     Shared Example Group: "non-empty report" called from ./spec/semgrep_image_spec.rb:269
     # /usr/bin/rspec:25:in `load'
     # /usr/bin/rspec:25:in `<main>'

<snip>

Finished in 8 minutes 56 seconds (files took 0.376 seconds to load)
427 examples, 32 failures
Failed examples:
rspec './spec/semgrep_image_spec.rb[1:2:4:1:1:1:1:1]' # running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report version is expected to exist "/builds/gitlab-org/security-products/analyzers/semgrep/tmp/test-22721/java/maven/running-image-with-...aven-build-tool-on-java-11-created-report-behaves-like-non-empty-report-version/gl-sast-report.json"
rspec './spec/semgrep_image_spec.rb[1:2:4:1:1:1:2:1]' # running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report scan is expected to exist "/builds/gitlab-org/security-products/analyzers/semgrep/tmp/test-22721/java/maven/running-image-with-...aven-build-tool-on-java-11-created-report-behaves-like-non-empty-report-version/gl-sast-report.json"
rspec './spec/semgrep_image_spec.rb[1:2:4:1:1:1:3:1]' # running image with test project with java when using maven build-tool on Java 11 created report behaves like non-empty report vulnerabilities is expected to exist "/builds/gitlab-org/security-products/analyzers/semgrep/tmp/test-22721/java/maven/running-image-with-...aven-build-tool-on-java-11-created-report-behaves-like-non-empty-report-version/gl-sast-report.json"


<snip>

I've also tested this image in this gemnasium pipeline and it succeeds as expected.

Edited Sep 12, 2024 by Adam Cohen

Fail with error when actual report does not exist

What does this MR do?

Further Details

What are the relevant issue numbers?

Testing

Merge request reports