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

Enhance Piracy detection by considering more methods #156

Merged
merged 4 commits into from
Aug 18, 2023

Conversation

lgoettgens
Copy link
Collaborator

@lgoettgens lgoettgens commented Jun 29, 2023

Resolves #155 and resolves #157.

The main change is a re-implementation of all_methods. Unfortunately, the old data source names(MyModule) was missing some callables (#157) and methods of non-imported functions that have been defined in a fully qualified way (#155). The proposed implementation works very similarly to Test.detect_ambiguities and Test.detect_unbound_args by iterating over all names of all currently loaded modules, and then filtering out the methods that are indeed defined in MyModule.

I furthermore added some tests displaying the new functionality. The main point here is that

# Assign them names in this module so they can be found by all_methods
a = Base.findfirst
b = Base.findlast
c = Base.findmax
d = Base.findmin

is no longer needed.

@codecov
Copy link

codecov bot commented Jul 1, 2023

Codecov Report

Merging #156 (9ae3c19) into master (5db17b8) will decrease coverage by 9.94%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #156      +/-   ##
==========================================
- Coverage   77.07%   67.13%   -9.94%     
==========================================
  Files          11       11              
  Lines         724      569     -155     
==========================================
- Hits          558      382     -176     
- Misses        166      187      +21     
Flag Coverage Δ
unittests 67.13% <100.00%> (-9.94%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/piracy.jl 90.16% <100.00%> (-0.89%) ⬇️

... and 10 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lgoettgens lgoettgens changed the title DO NOT MERGE: Add test for failing piracy example Enhance Piracy detection by considering more methods Jul 1, 2023
@lgoettgens lgoettgens marked this pull request as ready for review July 1, 2023 16:01
@lgoettgens
Copy link
Collaborator Author

Ping @fingolfin

@lgoettgens lgoettgens force-pushed the lg/piracy-callables branch from 8dbb2f5 to 9ae3c19 Compare July 18, 2023 08:32
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

I have no idea what this does exactly but I trust you thought this through.

@fingolfin fingolfin merged commit ff9761b into JuliaTesting:master Aug 18, 2023
@lgoettgens lgoettgens deleted the lg/piracy-callables branch August 18, 2023 12:34
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.

Piracy check misses callables Type-piracy check misses certain functions
2 participants