Resolve ambiguities

[KeithWM]

Let me know what you think. I took the liberty of rewriting the tests using ReTest, as it allows for much faster iteration in development.

Maybe besides tests that only test the perceived problem of potential method ambiguities, I should also include some tests that test the actual result of the operations.

[quinnj]

I'm personally not a fan of test Projects yet; they're undersupported by too many ecosystem tools at the moment.

[KeithWM]

I wasn't aware or ecosystems that don't support them. But it's OK as long as it works here on Github, right?

[quinnj]

Hmmm, why didn't CI run here?

[KeithWM]

I assumed CI was not run because I am somehow not approved to be the trigger. It made sense to me that CI would not be triggered by a PR from some random contributor.

[quinnj]

Sorry for the slow response here. Yeah, it's weird though because usually it pops up a button to ask if I want to approve CI for a first-time contributor. But I don't see anyway to kick off a CI run here.

That said, here's my thoughts:

• It feels hard to review this PR because of all the unrelated changes; I can't tell which tests are new and covering the changes because the entire files moved; it would be much easier to back out all the test file moving while we work thru the ambiguity issue and then do a follow up PR that's just about moving tests around
• I think Aqua/JET have some ways to automatically test for ambiguities? I can't quite remember the details, but it could be nice to hook into something automated like that to try and avoid future issues.

[KeithWM]

I have no idea why you aren't given the option to run the CI actions. Maybe Github knows this PR has a scope issue. ;-)

• I realized after creating the PR that I made a mistake in adjusting the tests so fundamentally in a PR that also includes other changes. I will create a new PR including only the first commit of this one, then it's almost entirely a "move".
• I was using Aqua.jl, that's how I found them. I could create an automated test for the ambiguities too, but that will fail until this PR (Resolve ambiguities #86) is merged.

[codecov]

Codecov Report

Patch coverage: 87.50% and project coverage change: +0.58 🎉

Comparison is base (bc8aba3) 94.81% compared to head (8ea444c) 95.40%.

❗ Current head 8ea444c differs from pull request most recent head b6af311. Consider uploading reports for the commit b6af311 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #86      +/-   ##
==========================================
+ Coverage   94.81%   95.40%   +0.58%     
==========================================
  Files           5        4       -1     
  Lines        1041     1000      -41     
==========================================
- Hits          987      954      -33     
+ Misses         54       46       -8     

Impacted Files	Coverage Δ
src/chainedvector.jl	96.66% <87.50%> (+0.02%)	⬆️

... and 2 files with indirect coverage changes

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

[quinnj]

Ok, finally got CI to run; can you take a look at the test failures? 1.3 is probably just too old, so we can bump that up to 1.6.

[KeithWM]

I've made some new commits to expand code coverage.

[quinnj]

Resolve the conflicts and I think this is ready to go?

[KeithWM]

I think I resolved the conflict. CI/CD should be run to confirm. (I should maybe have done a FF on my "main" rather than a merge commit from this main, but alas)

[nilshg]

Bump - is there anything holding this up?