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

Fix/issue/512 #515

Merged
merged 1 commit into from May 30, 2022
Merged

Fix/issue/512 #515

merged 1 commit into from May 30, 2022

Conversation

shifqu
Copy link
Contributor

@shifqu shifqu commented Sep 28, 2021

Description

Enrich global context in eval calls in CodeEvaluator._assert_code

Motivation behind this PR?

When writing an assert statement in scanapi which uses a list (or any other) comprehension, the code would fail.

What type of change is this?

Bugfix

Checklist

  • I have added a changelog entry / my PR does not need a new changelog entry. Instructions.
  • I have added/updated unit tests. Instructions.
  • New and existing unit tests pass locally with my changes. Instructions
  • I have self-documented code my changes by adding docstring(s) and comment(s). Instructions
  • Current PR does not significantly decrease the code coverage and docstring coverage.
  • My code follows the style guidelines of this project.
  • I have run ScanAPI locally and manually tested my changes. Instructions.
  • I have squashed my commits. Instructions.

Issue

Closes #512

@shifqu shifqu requested review from a team as code owners September 28, 2021 14:03
@github-actions github-actions bot added the First Contribution First contribution to the project. label Sep 28, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for supporting ScanAPI, and congratulations on your first contribution! A project committer will shortly review your contribution.

In the mean time, if you haven't had a chance please skim over the First Pull Request Guide which all pull requests must adhere to.

We hope to see you around!

@shifqu
Copy link
Contributor Author

shifqu commented Oct 8, 2021

I had no idea black did not run on saving (I have this enabled on all my repositories by default and thus, did not pay real attention to it).

The black linting should pass now.

As for the DeepSource, it complains about using eval. Which it does so correctly, but this is the bread and butter of scanapi, so I was wondering how to add an exception to this hook.

Copy link
Member

@astenstrasser astenstrasser left a comment

Choose a reason for hiding this comment

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

Hey @shifqu, thanks for the PR.
Looks great, nice approach :)

I notice there are some other commits included in the PR, not only your changed.
Could you please adjust it so we can merge?
Thanks!!

@shifqu
Copy link
Contributor Author

shifqu commented May 27, 2022

Requested changes have been pushed. I also squashed the commits

@astenstrasser astenstrasser merged commit ea20ad5 into scanapi:main May 30, 2022
camilamaia pushed a commit that referenced this pull request May 31, 2022
Co-authored-by: shifqu <shifqu@softllama.net>
@camilamaia camilamaia mentioned this pull request Jun 1, 2022
@camilamaia
Copy link
Member

Congrats on your first merged PR! 🌟 Thank you very much!

I am going to send you an invite to join the ScanAPI org on GitHub 🚀 We invite everyone that has contributed with a merged PR in any of our repositories. Here you can check our Contributing Guidelines so you can understand better how it works.

Check your email/GitHub notifications to find the invite. If you accept it, you will also be able to clone directly our repositories, without needing to fork them.

Welcome on board and once again, thank you! 🙇‍♀️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First Contribution First contribution to the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using comprehensions in assert statements causes a NameError
3 participants