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

Try out PkgEval #335

Closed
wants to merge 2 commits into from
Closed

Try out PkgEval #335

wants to merge 2 commits into from

Conversation

maleadt
Copy link

@maleadt maleadt commented Jan 9, 2023

For #334, but including a version bump (which is required as Nanosoldier will actually register this package in a temporary registry).

Ref JuliaCI/Nanosoldier.jl#154; this will probably require a couple of iterations to get a successful/satisfactory run.

@maleadt
Copy link
Author

maleadt commented Jan 9, 2023

@nanosoldier runtests()

@nanosoldier
Copy link

Update on PkgEvalJob 89924db vs. fa775b3: Accepted

@nanosoldier
Copy link

Update on PkgEvalJob 89924db vs. fa775b3: Running

@maleadt
Copy link
Author

maleadt commented Jan 9, 2023

... I forgot the subdir argument:

@nanosoldier runtests(subdir="SnoopPrecompile")

@nanosoldier
Copy link

Update on PkgEvalJob 89924db vs. fa775b3: Accepted

@nanosoldier
Copy link

Update on PkgEvalJob 89924db vs. fa775b3: Running

@nanosoldier
Copy link

Your job failed: could not register new version (Version 1.0.2 already exists).

@maleadt
Copy link
Author

maleadt commented Jan 9, 2023

At least this verifies that the new error reporting is working fine 🙂

@nanosoldier runtests(subdir="SnoopPrecompile")

@nanosoldier
Copy link

Update on PkgEvalJob af9918f vs. fa775b3: Accepted

@nanosoldier
Copy link

Update on PkgEvalJob af9918f vs. fa775b3: Running

@codecov
Copy link

codecov bot commented Jan 9, 2023

Codecov Report

Base: 82.26% // Head: 83.57% // Increases project coverage by +1.31% 🎉

Coverage data is based on head (af9918f) compared to base (fa775b3).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #335      +/-   ##
==========================================
+ Coverage   82.26%   83.57%   +1.31%     
==========================================
  Files          17       17              
  Lines        2176     2186      +10     
==========================================
+ Hits         1790     1827      +37     
+ Misses        386      359      -27     
Impacted Files Coverage Δ
src/deep_demos.jl 58.97% <0.00%> (-3.19%) ⬇️
src/parcel_snoopi_deep.jl 90.01% <0.00%> (+0.53%) ⬆️
src/invalidations.jl 73.76% <0.00%> (+1.38%) ⬆️
src/parcel_snoopi.jl 95.00% <0.00%> (+3.33%) ⬆️
SnoopPrecompile/src/SnoopPrecompile.jl 94.11% <0.00%> (+3.49%) ⬆️
src/parcel_snoopc.jl 78.15% <0.00%> (+5.04%) ⬆️
SnoopCompileCore/src/snoopi.jl 55.73% <0.00%> (+8.46%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@nanosoldier
Copy link

Your package evaluation job has completed - possible new issues were detected.
A full report can be found here.

@maleadt
Copy link
Author

maleadt commented Jan 9, 2023

Hmm, all failures are due to the modified SnoopPrecompile failing to load:

SnoopPrecompile [66db9d55-30c0-4569-8b51-7e840670fc0c]

Error: Missing source file for SnoopPrecompile [66db9d55-30c0-4569-8b51-7e840670fc0c]

@KristofferC Any thoughts on this failure? I haven't seen it before. PkgEval is using a custom registry and actually registering v1.0.3.

@KristofferC
Copy link
Collaborator

Can you put the custom registry somewhere accessible and we can see if it repros when we use that as our only registry?

@maleadt
Copy link
Author

maleadt commented Jan 10, 2023

Figured out with the help of @KristofferC that this was an issue with the modified registry. Has been fixed, but PkgEval is busy right now, so I'll trigger again once I've had the change to update the system.

@maleadt
Copy link
Author

maleadt commented Jan 10, 2023

@nanosoldier runtests()

@nanosoldier
Copy link

Update on PkgEvalJob maleadt/SnoopCompile.jl@af9918f: Running

@nanosoldier
Copy link

Update on PkgEvalJob maleadt/SnoopCompile.jl@af9918f: Accepted

@maleadt
Copy link
Author

maleadt commented Jan 10, 2023

Forgot subdir again:

@nanosoldier runtests(subdir="SnoopPrecompile")

@nanosoldier
Copy link

Update on PkgEvalJob maleadt/SnoopCompile.jl@af9918f: Accepted

@nanosoldier
Copy link

Update on PkgEvalJob maleadt/SnoopCompile.jl@af9918f: Running

@nanosoldier
Copy link

Your package evaluation job has completed - no issues were detected.
A full report can be found here.

@maleadt
Copy link
Author

maleadt commented Jan 10, 2023

That last report looks great (both from a PkgEval/Nanosoldier and SnoopCompile perspective). Feel free to trigger Nanosoldier now whenever it's useful. Note that you currently have to bump the version before triggering, but I'll look into removing that requirement this week, so you can assume that it will be possible to trigger Nanosoldier anywhere on this repository.

@maleadt maleadt closed this Jan 10, 2023
@maleadt maleadt deleted the patch-1 branch January 10, 2023 17:58
@timholy
Copy link
Owner

timholy commented Jan 10, 2023

Thanks so much! Just to check that I understand: I just use the same invocation you did, @nanosoldier runtests(subdir="SnoopPrecompile"), after bumping the version in SnoopPrecompile's Project.toml?

@maleadt
Copy link
Author

maleadt commented Jan 11, 2023

Just to check that I understand: I just use the same invocation you did, @nanosoldier runtests(subdir="SnoopPrecompile"), after bumping the version in SnoopPrecompile's Project.toml?

Yes, and you can leave out the version bump (I'm currently working on automating that).

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.

4 participants