frictionlessdata / datapackage-go

A Go library for working with Data Package.
MIT License
21 stars 5 forks source link

Introduce windows build #30

Closed danielfireman closed 2 years ago

danielfireman commented 2 years ago

Overview

We tried and some tests are failing and @danielfireman currently does not have a good windows setup to debug/fix them.

I will be happy to help if someone is interested on take this.


Please preserve this line to notify @danielfireman (lead of this repository)

loleg commented 2 years ago

@danielfireman I'd be happy to at least duplicate the tests. Can you share current status or any tips?

danielfireman commented 2 years ago

@danielfireman I'd be happy to at least duplicate the tests.

Thanks a lot, @loleg !!!

Can you share current status or any tips?

It should be only a matter of making sure you have a recent go version 1.16+, cloning the repo and running tests:

go test ./...

The go runtime should automatically download the dependencies and run all tests.

Please let me know if you run into any problems and if you have some time this week to pair programming to find and fix the root cause.

loleg commented 2 years ago

I don't know if my setup is "ideal", but I have the latest stable Go, Git and VS Code installed with a workspace for the project. I have a couple of tests failing, for reasons not immediately clear:

(1) In ExampleLoad_readAll

got:
[]
want:
[[foo] [bar]]

(2) In ExampleLoad_readRaw

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x0 pc=0xdc1180]

goroutine 1 [running]:
testing.(*InternalExample).processRunResult(0xc0005cbc68, {0x0, 0x0}, 0x0?, 0x0, {0xe0d600, 0x10fb030})
        C:/Program Files/Go/src/testing/example.go:91 +0x4e5
testing.runExample.func2()
        C:/Program Files/Go/src/testing/run_example.go:59 +0x11c
panic({0xe0d600, 0x10fb030})
        C:/Program Files/Go/src/runtime/panic.go:838 +0x207
github.com/frictionlessdata/datapackage-go/datapackage.ExampleLoad_readRaw()
        d:/Localdev/frictionless/datapackage-go/datapackage/package_test.go:81 +0x300
testing.runExample({{0xe6921a, 0x13}, 0xeaec68, {0xe782f6, 0x2f}, 0x0})
        C:/Program Files/Go/src/testing/run_example.go:63 +0x28d
testing.runExamples(0xc0005cbe58, {0x1102760?, 0x5, 0x1a?})
        C:/Program Files/Go/src/testing/example.go:44 +0x186
testing.(*M).Run(0xc00005b180)
        C:/Program Files/Go/src/testing/testing.go:1721 +0x689
main.main()
        _testmain.go:107 +0x1aa
FAIL    github.com/frictionlessdata/datapackage-go/datapackage  2.663s

(3) In TestLoad

=== RUN   TestLoad/LocalZipWithSubdirs
        d:\Localdev\frictionless\datapackage-go\datapackage\package_test.go:518: [data
                                                                                      oo.csv] != [data/foo.csv]
drunkcod commented 2 years ago

@loleg I stumbled into these problems yesterday and after a bit of bumbling around I concluded the source of all the failures to be related to basePath handling and how resource paths are joined.

One of them also fail due to the osSeparator character for windows ending up being a escape sequence in the test data.

I could get it to work for my purposes with the following: https://github.com/drunkcod/datapackage-go/commit/c9e04d11500c785f4ce29252e27a3cd9e888e324

I'm hesitant to submit it as a PR since this is my very first encounter with go (I wanted to see how datapackages worked in that environment) and since you're obviously already on the ball and likely have a much better position (time and knowledge)

tldr; it's the paths!

danielfireman commented 2 years ago

@loleg I stumbled into these problems yesterday and after a bit of bumbling around I concluded the source of all the failures to be related to basePath handling and how resource paths are joined.

Spot on, @drunkcod !

One of them also fail due to the osSeparator character for windows ending up being a escape sequence in the test data.

Exactly! I had quick access to a windows machine and found that out recently. Unfortunately, I didn't have the time to fix it.

I could get it to work for my purposes with the following: drunkcod@c9e04d1

I'm hesitant to submit it as a PR since this is my very first encounter with go (I wanted to see how datapackages worked in that environment) and since you're obviously already on the ball and likely have a much better position (time and knowledge)

I gave a quick look at the branch and it looks nice. Please send the PR on my way and I will be happy to provide constructive feedback to trim the edges. It would be awesome to finally get the windows build fixed!

tldr; it's the paths!

danielfireman commented 2 years ago

I don't know if my setup is "ideal", but I have the latest stable Go, Git and VS Code installed with a workspace for the project. I have a couple of tests failing, for reasons not immediately clear ...

Thanks a lot, @loleg for running setting up the environment and giving the tests a try! Hoping to see @drunkcod PR fixing those two problems you've bumped into.

loleg commented 2 years ago

No worries. I tried also to use url.ParseRequestURI(p) in getBasepath as a quick debug did show me the file path was treated as URL.

loleg commented 2 years ago

All tests passing on drunkcod/win-fixes - I think the regexp solution is reasonable, but this might be a bit better as it uses standard libs: https://go.dev/play/p/sIY_yTwV978 - I'll add a comment.

danielfireman commented 2 years ago

Excellent point, @loleg! Already replied to it with slight modifications. @drunkcod could you please take a look? Please let me know if you have any questions.