search

boot-clj / boot

Build tooling for Clojure.
https://boot-clj.github.io/
Eclipse Public License 1.0
1.75k stars 180 forks source link

Make pod tests pass, and run them from `make` #567

Closed bhagany closed 7 years ago

bhagany commented 7 years ago

Problems with these tests were discovered while working on #566, so I figured I'd take a shot at fixing them. I elected to change the code as little as possible, and change the tests instead, but I'm not super confident I made the right decision in every case. Criticism welcome.

bhagany commented 7 years ago

Hrm, I take it Appveyor runs on Windows...

arichiardi commented 7 years ago

Great thanks!

bhagany commented 7 years ago

alright, my AppVeyor fix worked :)

arichiardi commented 7 years ago

Looks very good to me :)

bhagany commented 7 years ago

Okay @arichiardi, I hope you'll like this. It occurred to me that io/file will construct paths with platform-specific separators, so we can rely on AppVeyor tests to test Windows paths without doing it explicitly. CI passed, so all is well!

I also removed the test for AssertionError, since the :pre check for nil on relative-to makes it redundant. Let me know what you think.

arichiardi commented 7 years ago

Perfect! I think now it looks way way simpler. Maybe the original author of this ns can also chime in. In any case, good job!

bhagany commented 7 years ago

@arichiardi I just realized github is still saying that you're requesting changes. Is that still true? If so, I'm not sure what they are.

arichiardi commented 7 years ago

Uhm no, i am good, let me see if I can cancel the review.

bhagany commented 7 years ago

Fantastic, thanks :)

alandipert commented 7 years ago

:metal: :guitar: :metal: