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

Mutalk handles parametrized tests correctly #95

Merged

Conversation

DurieuxPol
Copy link
Collaborator

Fixes #88
Now MTAnalysis retrieves correctly all tests cases from parametrized tests.
I also refactored a bit MTTestCaseReference so that it now has the test case as instance variable rather than the class and the selector.

Copy link

@PalumboN PalumboN left a comment

Choose a reason for hiding this comment

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

Cool! 🚀

I left some questions and minor comments.

I have doubts about the use of the selector: method... It feels like a query method that have effect => bad method.
Just to take care.

src/MuTalk-Model/MTTestCaseReference.class.st Show resolved Hide resolved
Comment on lines 47 to 50
MTTestCaseReference >> method [
^ class >> selector

^ testCase class >> testCase selector
]
Copy link

Choose a reason for hiding this comment

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

Maybe would be nice to have in the test case, and do testCase method or testCase methodTest or similar.

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 agree but it means changing the TestCase class, which is a different thing.
For now I guess this solution is fine

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea, maybe it's worth to do a PR to Pharo? @DurieuxPol ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes we can do that

src/MuTalk-Model/MTTestCaseReference.class.st Outdated Show resolved Hide resolved
src/MuTalk-Tests/MTAnalysisTest.class.st Show resolved Hide resolved
@guillep
Copy link
Contributor

guillep commented Mar 14, 2024

Thanks @DurieuxPol and @PalumboN for the review!

@guillep guillep merged commit 99c63f8 into pharo-contributions:master Mar 14, 2024
4 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.

Mutalk doesn't handle parametrized tests
3 participants