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

[STAL-1686] Follow-up for handling rules marked as is_testing #349

Merged
merged 6 commits into from
May 7, 2024

Conversation

modernplumbing
Copy link
Contributor

@modernplumbing modernplumbing commented May 6, 2024

What problem are you trying to solve?

When releasing version 0.3.3 of the analyzer, we started having 422s. This was due to the front-end not sending is_testing if is_testing was unset. The IDE integration would have the same issue. This PR does the following to address this issue:

  1. Adds a default value (false) if is_testing is empty in the Rule struct.
  2. Remove the is_testing field from the ServerRule struct.
  3. Updates the test-production-rules.py integration test to expose the error if it fails, rather than just returned non-zero exit status 1.
  • However, there could possibly be a cleaner way of surfacing the error. The current output is
Testing ruleset python-best-practices on the CLI
   Testing rule special-methods-arguments
Traceback (most recent call last):
  File "/home/runner/work/datadog-static-analyzer/datadog-static-analyzer/misc/test-production-rules.py", line 208, in test_ruleset_cli
    subprocess.run(shlex.split(cmd), check=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
  File "/usr/lib/python3.10/subprocess.py", line 526, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['/home/runner/work/datadog-static-analyzer/datadog-static-analyzer/target/release/datadog-static-analyzer', '-r', '/tmp/tmp7tcyto2m/special-methods-arguments/next-with-error.py.datadog.rules', '-i', '/tmp/tmp7tcyto2m/special-methods-arguments', '-o', '/tmp/tmp7tcyto2m/special-methods-arguments/next-with-error.py.results', '-f', 'json']' returned non-zero exit status 1.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/runner/work/datadog-static-analyzer/datadog-static-analyzer/misc/test-production-rules.py", line 243, in <module>
    test_ruleset_cli(ruleset)
  File "/home/runner/work/datadog-static-analyzer/datadog-static-analyzer/misc/test-production-rules.py", line 210, in test_ruleset_cli
    raise RuntimeError("command '{}' return with error (code {}): {}".format(e.cmd, e.returncode, e.output))
RuntimeError: command '['/home/runner/work/datadog-static-analyzer/datadog-static-analyzer/target/release/datadog-static-analyzer', '-r', '/tmp/tmp7tcyto2m/special-methods-arguments/next-with-error.py.datadog.rules', '-i', '/tmp/tmp7tcyto2m/special-methods-arguments', '-o', '/tmp/tmp7tcyto2m/special-methods-arguments/next-with-error.py.results', '-f', 'json']' return with error (code 1): b'Error: cannot read ruleset from file\n\nCaused by:\n    missing field `is_testing` at line 1 column 2486\n'

What is your solution?

Testing

Followed the procedure here: #339

@modernplumbing modernplumbing changed the title [STAL-1686] Set default for is_testing [STAL-1686] Set default for is_testing and remove is_testing field from ServerRule May 7, 2024
@modernplumbing modernplumbing requested a review from juli1 May 7, 2024 15:21
@modernplumbing modernplumbing changed the title [STAL-1686] Set default for is_testing and remove is_testing field from ServerRule [STAL-1686] Follow-up for handling rules marked as is_testing May 7, 2024
@modernplumbing modernplumbing merged commit 0fb157c into main May 7, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants