Not so quick fix to allow "/" at end of validator URL, plus fixes and tests for --as_type

[ml-evs]

Handles an edge case that got lost in the last PR, i.e. handling slashes at the end of the URL provided at the CLI.

[ml-evs]

Annoyingly missed this from the last PR... sorry @CasperWA!

[codecov]

Codecov Report

Merging #267 into master will increase coverage by 0.77%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master     #267      +/-   ##
==========================================
+ Coverage   89.15%   89.92%   +0.77%     
==========================================
  Files          54       54              
  Lines        2241     2244       +3     
==========================================
+ Hits         1998     2018      +20     
+ Misses        243      226      -17     

Flag	Coverage Δ
#unittests	89.92% <83.33%> (+0.77%)	⬆️

Impacted Files	Coverage Δ
optimade/validator/validator.py	74.03% <83.33%> (+5.94%)	⬆️
optimade/server/routers/utils.py	97.70% <0.00%> (+0.76%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5cc923...67b7dd5. Read the comment docs.

[CasperWA]

I guess I wasn't thorough enough... I'll be even MORE nitpicky now then ! 😆

No but seriously, while is nicer ... 😅

[ml-evs]

No but seriously, while is nicer ... sweat_smile

👍 We can just squash away my sarcastic commit message :P

[CasperWA]

Oh no! What about id's with / in them, or rather, ending with /?

[ml-evs]

Oh no! What about id's with / in them, or rather, ending with /?

I don't think we allow this at the moment right? You can only validate endpoints with multi-entry response types, not individual entries with the entry type

[CasperWA]

Oh no! What about id's with / in them, or rather, ending with /?

I don't think we allow this at the moment right? You can only validate endpoints with multi-entry response types, not individual entries with the entry type

That's okay then.

[ml-evs]

Oh no! What about id's with / in them, or rather, ending with /?

I don't think we allow this at the moment right? You can only validate endpoints with multi-entry response types, not individual entries with the entry type

That's okay then.

It looks like this has messed something else up though, don't merge this yet...

[ml-evs]

The --as_type functionality is so brittle as currently implemented, but I've patched it back together for now. It's unfortunately hard to add tests for it too, as we there are slightly different code paths when querying a remote vs providing a client (as the tests do). I've managed to mock the underlying Client class from the validator so we get more realistic testing. I think it would be sensible to do this in the other tests, so we can eventually deprecate the ability to pass the validator a Client-like object anyway (which is where a lot of these bugs have come from).

[CasperWA]

The --as_type functionality is so brittle as currently implemented, but I've patched it back together for now. It's unfortunately hard to add tests for it too, as we there are slightly different code paths when querying a remote vs providing a client (as the tests do). I've managed to mock the underlying Client class from the validator so we get more realistic testing. I think it would be sensible to do this in the other tests, so we can eventually deprecate the ability to pass the validator a Client-like object anyway (which is where a lot of these bugs have come from).

Seems reasonable.
We might need to reconsider what parameters the validator CLI should have as well and whether --as-type is relevant. Although it may be, since one could have a special endpoint that still serves "valid" structures.

[ml-evs]

Also turns out we definitely do allow single entry response validation, I am talking so much rubbish today...

[ml-evs]

The --as_type functionality is so brittle as currently implemented, but I've patched it back together for now. It's unfortunately hard to add tests for it too, as we there are slightly different code paths when querying a remote vs providing a client (as the tests do). I've managed to mock the underlying Client class from the validator so we get more realistic testing. I think it would be sensible to do this in the other tests, so we can eventually deprecate the ability to pass the validator a Client-like object anyway (which is where a lot of these bugs have come from).

Seems reasonable.
We might need to reconsider what parameters the validator CLI should have as well and whether --as-type is relevant. Although it may be, since one could have a special endpoint that still serves "valid" structures.

Yeah the whole reason for this initially was to validate nomad with their calculations<->structures renaming

[CasperWA]

Also turns out we definitely do allow single entry response validation, I am talking so much rubbish today...

Hehe. You need to go get a beer and a sleep 😄

[CasperWA]

Look's good!
A couple of questions/comments :)

[CasperWA]

Thanks for not so quick quick-fix @ml-evs ! 😄