kemalcr / kemal

Fast, Effective, Simple Web Framework
https://kemalcr.com
MIT License
3.65k stars 189 forks source link

Test Windows CI #657

Closed mamantoha closed 2 months ago

mamantoha commented 1 year ago

This was just a test for Windows CI

Finished in 49.14 seconds
121 examples, 13 failures, 16 errors, 0 pending
mamantoha commented 1 year ago

to fix the check_ameba workflow go to Setting -> Actions -> General and check "Read and write permissions"

image

sdogruyol commented 1 year ago

Thanks a lot @mamantoha 👍 Could you please tell me why do we need the permissions for ameba

mamantoha commented 1 year ago

@sdogruyol I don't know why https://github.com/crystal-ameba/github-action requires write permissions

sdogruyol commented 1 year ago

@mamantoha that's already enabled, do I need to do anything else?

mamantoha commented 1 year ago

@sdogruyol I don't understand why check_ameba fails. It works fine on my fork.

sdogruyol commented 1 year ago

Just reran the workflow and it failed with the same error. Not sure what's wrong with permissions 🤷

HertzDevil commented 1 year ago

The Kemal::StaticFileHandler specs are failing because of this part:

https://github.com/kemalcr/kemal/blob/c995a2a9711d72d90b347683007b45bbaf495317/src/kemal/static_file_handler.cr#L30-L38

Both @public_dir and expanded_path contain the drive name, so file_path becomes something like C:\...\C:\.... Replacing the first line with just expanded_path = request_path makes all specs pass. I have not dug far enough to understand if the File.expand_path call has any significance on other platforms.

straight-shoota commented 1 year ago

StaticFileHandler probably just needs to be aligned with stdlib's implementation.

HertzDevil commented 1 year ago

That part in stdlib is:

    request_path = Path.posix(request_path)
    expanded_path = request_path.expand("/")

    file_path = @public_dir.join(expanded_path.to_kind(Path::Kind.native))
mamantoha commented 1 year ago

Hi @HertzDevil @straight-shoota I don't have a Windows environment to test this build locally. Can someone else take over and continue working on this pull request? Thank you.

mamantoha commented 2 months ago

Closed in favor of https://github.com/kemalcr/kemal/pull/690