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

General result shows filtered tests #84

Merged
merged 11 commits into from
Mar 14, 2024

Conversation

DurieuxPol
Copy link
Collaborator

@DurieuxPol DurieuxPol commented Feb 22, 2024

Also shows the reason and the code of the selected test.

image

@DurieuxPol DurieuxPol changed the title General result shows filtered test General result shows filtered tests Feb 22, 2024
@guillep
Copy link
Contributor

guillep commented Feb 22, 2024

Great!

There is SpCodePresenter that supports some syntax highlighting and other navigation features. Maybe you can use that?

Copy link
Contributor

@guillep guillep left a comment

Choose a reason for hiding this comment

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

Thanks @DurieuxPol ! Could you do the renames? then for the big comment we can discuss tomorrow :)

into: [ :accumulator :test |
accumulator add:
test -> testFilter filteredTestReason.
accumulator ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm this method is strange :).

  • First, this inject:into: could be replaced by a collect: (a simple map) no?
  • Second, the objective of this method is to store the tests + reason, right?
    I have one question here. All filtered tests have the same reason right now, right? I wonder why the test filter does not return the "tuple" already instead of creating it in here... It's the filter that decides to filter, then it could be the filter that attaches this to the test :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, I refactored the test filters in this direction

@@ -536,6 +563,7 @@ MTAnalysis >> generateResults [
| tests |
mutantResults := OrderedCollection new.
tests := testFilter filterTests: testCases.
self filteredTestsFrom: (testCases copyWithoutAll: tests).
Copy link
Contributor

Choose a reason for hiding this comment

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

May I propose to rename tests into filteredTests? :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed !

arg1 , ' ' , arg2 ].

reasonString , pragmaString , '>' ]
]
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is weird... why is it attaching > at the end of the string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because reasonString is the start of the string, and it contains <.
It's because I wanted the pragma to be between <>, like this <pragma>

Copy link
Contributor

@guillep guillep left a comment

Choose a reason for hiding this comment

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

Thanks @DurieuxPol !

@guillep guillep merged commit d742fbf into pharo-contributions:master Mar 14, 2024
4 checks passed
@DurieuxPol DurieuxPol mentioned this pull request Mar 15, 2024
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.

2 participants