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

Add pytest entrypoints #151

Merged
merged 4 commits into from
Feb 23, 2024
Merged

Conversation

khatchad
Copy link
Collaborator

@khatchad khatchad commented Feb 23, 2024

Modeled after the turtle entrypoints. See http://pytest.org.

* Initial fix.

* Use Guava to merge the iterables.

Looks a lot cleaner.

* Cleaning.

* Log entrypoints.

* Enhance.

* Add pytest calls to methods test.

* Update .gitignore.

* Move to new class.

* Add pytest test class with an explicit ctor.

* Simply assertions.
@khatchad khatchad marked this pull request as ready for review February 23, 2024 16:54
@khatchad khatchad requested a review from msridhar February 23, 2024 16:54
Copy link
Member

@msridhar msridhar left a comment

Choose a reason for hiding this comment

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

One comment but I don't need to check again

Comment on lines 55 to 71
FieldReference global =
FieldReference.findOrCreate(
PythonTypes.Root,
Atom.findOrCreateUnicodeAtom(
"global "
+ getMethod()
.getDeclaringClass()
.getName()
.toString()
.substring(
1,
getMethod()
.getDeclaringClass()
.getName()
.toString()
.lastIndexOf('/'))),
PythonTypes.Root);
Copy link
Member

Choose a reason for hiding this comment

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

There seem to be some repeated sub-expressions here. This would be more readable if those were stored in locals.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I more or less copied this and adapted it from the Turtle case. I'll have a look. I admittedly don't know everything that is going on here but it seems to work well.

Copy link
Member

Choose a reason for hiding this comment

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

Ok no worries. Just a comment on readability, I'm sure it works.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK I gave a it a shot.

@khatchad khatchad added the enhancement New feature or request label Feb 23, 2024
@khatchad khatchad enabled auto-merge (squash) February 23, 2024 17:30
@khatchad khatchad merged commit f1427ae into wala:master Feb 23, 2024
1 check passed
@khatchad khatchad deleted the contrib_pytest_entrypoints branch February 23, 2024 17:31
@khatchad khatchad mentioned this pull request Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants