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.

Sign up for GitHub

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

Jump to bottom

Enhance file exclusion by ignoring Package.swift and make it more folder aware #258

Merged
merged 4 commits into from
Jan 11, 2024
Merged

Enhance file exclusion by ignoring Package.swift and make it more folder aware #258

merged 4 commits into from
Jan 11, 2024

Conversation

Nikoloutsos
Copy link
Contributor

@Nikoloutsos Nikoloutsos commented Dec 13, 2023 •
edited
Loading

Hello, this PR implements what is proposed here #250 and #259

  • Start excluding Package.swift by default
  • Change the default exclusion paths to take / into consideration. Before SpecialFeatureViewModel was ignored because it cointained the substring "Spec". Now it is not ignored 🥳 . This feature gives the developer more flexibility

@Nikoloutsos Nikoloutsos mentioned this pull request Dec 14, 2023
Closed
@Nikoloutsos Nikoloutsos changed the title Exclude (by default) the mutation on Package.swift Enhance file exclusion by ignoring Package.swift and making exclusion more specific Dec 14, 2023
@Nikoloutsos Nikoloutsos changed the title Enhance file exclusion by ignoring Package.swift and making exclusion more specific Enhance file exclusion by ignoring Package.swift and making default exclusion paths more specific Dec 14, 2023
@Nikoloutsos Nikoloutsos force-pushed the feature/exclude-spm-package-swift-from-mutations branch from 7ad9f5d to 9342a49 Compare December 14, 2023 09:00
@Nikoloutsos Nikoloutsos changed the title Enhance file exclusion by ignoring Package.swift and making default exclusion paths more specific Enhance file exclusion by ignoring Package.swift and make it more folder aware Dec 14, 2023
@Nikoloutsos
Copy link
Contributor Author

Nikoloutsos commented Dec 14, 2023 •
edited
Loading

Will check later why the acceptance test fails on test_xcodeFormat. Locally it passes 🤔

Update:
It looks like this test is flaky. It is not introduced in this PR.
I will probably investigate it more in a separate PR.

@Nikoloutsos Nikoloutsos force-pushed the feature/exclude-spm-package-swift-from-mutations branch 2 times, most recently from 9342a49 to 90315fc Compare December 15, 2023 00:49
@rakaramos
Copy link
Member

hey @Nikoloutsos thanks for the PR, I'm reviewing this soon.

rakaramos
rakaramos reviewed Dec 28, 2023
View reviewed changes
Sources/muterCore/MuterRunSteps/DiscoverSourceFiles.swift
@@ -74,6 +77,7 @@ private extension DiscoverSourceFiles {
root: rootPath
)
)
.map { String($0.dropFirst(2)) }
Copy link
Member

Choose a reason for hiding this comment

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

interesting, why is this needed?

Copy link
Contributor Author

@Nikoloutsos Nikoloutsos Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

some lines above I am doing .map { "./" + $0 }. This line removes the ./.
That way the filepaths returned from the DiscoverSourceFiles step will be the same as it was before.

rakaramos
rakaramos reviewed Dec 28, 2023
View reviewed changes
Repositories/ExampleSPM/Package.swift Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I'm not seeing anything being used in the tests in regards to this project, am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rakaramos Yes it is not used from the tests but we could. I added it because it is the only SPM package inside repo that you can run muter. I can drop it.

@rakaramos
Copy link
Member

@Nikoloutsos will it work if the client project has a file named Package.swift that is not the SPM manifest?

@Nikoloutsos
Copy link
Contributor Author

Nikoloutsos commented Jan 3, 2024 •
edited
Loading

@rakaramos Happy new year 🥳
At this point the filtering only checks if the file path contains a specific substring.

MyProject/Package.swift --> Will be ignored by default 🟢
MyProject/UtilPackage.swift --> Will NOT be ignored since /Package.swift is not a substring

So yes if somebody is using Package.swift as the name of a file it will be ignored. But IIRC this filename is preserved so it is not a best practice to use it.

@Nikoloutsos Nikoloutsos force-pushed the feature/exclude-spm-package-swift-from-mutations branch from 90315fc to 953fb15 Compare January 3, 2024 21:09
@Nikoloutsos
Copy link
Contributor Author

@rakaramos
I've dropped the ExampleSPM folder. Let me know if there is anything more you would like to discuss.

@rakaramos
Copy link
Member

@Nikoloutsos its fine for now, we can revisit this later and do some extra checks to make sure that we are not ignoring something that we should not.
thanks!

@rakaramos rakaramos merged commit d44e78d into muter-mutation-testing:master Jan 11, 2024
1 check passed
@Nikoloutsos Nikoloutsos deleted the feature/exclude-spm-package-swift-from-mutations branch January 21, 2024 11:43
@rakaramos rakaramos mentioned this pull request Feb 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rakaramos rakaramos Awaiting requested review from rakaramos

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Nikoloutsos @rakaramos