Skip to content
This repository has been archived by the owner on Mar 22, 2022. It is now read-only.

Ignore img directory #5

Merged
merged 1 commit into from
Jun 16, 2016
Merged

Conversation

ryanplusplus
Copy link
Member

This is needed to support exercism/exercism#2925

@ryanplusplus
Copy link
Member Author

ryanplusplus commented Jun 15, 2016

I don't know how to actually run these tests and was hoping that Travis would do it for me but no such luck. I'm able to run the top level tests, but not the tests inside of configlet/ (ie: where these changes exist). What am I doing wrong?

The output of go get in configlet/ is:

go install: no install location for directory /home/ryan/go/configlet/configlet outside GOPATH

My GOPATH is set to ~/go and GOBIN is set to ~/go/bin.

@kytrinyx
Copy link
Member

If the GOPATH is ~/go, then Go is going to expect configlet to be at ~/go/github.com/exercism/configlet

I wrote a blog post about contributing to go projects (it has some differences compared to other languages): https://splice.com/blog/contributing-open-source-git-repositories-go/

I'll pull this down and check (and get travis to run the tests. Good catch.)

@ryanplusplus
Copy link
Member Author

Thanks @kytrinyx, I'll give that a read.

@kytrinyx
Copy link
Member

I'm getting two failures on this. The first one is just ordering (maybe we should just order both slices before checking equality).

--- FAIL: TestIgnoredDirsIsUnique (0.00s)
        Location:       config_test.go:31
        Error:          Not equal: []string{".git", "bin", "img", "fig", "ignored"} != []string{".git", "bin", "fig", "ignored", "img"}

--- FAIL: TestSlugs (0.00s)
        track_test.go:128: Expected len(slugs:map[crystal:{} melanite:{} sapphire:{} .git:{} bin:{} ignored:{} img:{} amethyst:{} no-such-dir:{} opal:{} diamond:{} pearl:{} beryl:{}])==13 to equal len(expected:[.git amethyst beryl bin crystal diamond ignored melanite no-such-dir opal pearl sapphire])==12

@kytrinyx
Copy link
Member

OK, if you rebase onto master that should fix the first test failure.

@kytrinyx
Copy link
Member

I've also tweaked the logic to ignore hidden directories implicitly (skipping hidden directories when detecting them instead of adding them to an explicit list of directories to ignore). This is to support running the project on travis, since it doesn't include the .git directory there, which throws the tests.

This will probably cause conflicts—I think it should be pretty straight forward to remove any mention of .git in the test expectations, though.

@kytrinyx
Copy link
Member

OK, I've also gotten this to run on travis, so if you rebase onto master it should give you proper feedback.

@ryanplusplus
Copy link
Member Author

ryanplusplus commented Jun 16, 2016

Okay, I'm now able to run tests, but it looks like master does not pass tests. There's a problem detecting hidden directories in evaluate_test.go that does not seem to manifest itself in track_test.go. I wasn't able to figure out why this is or how to fix it. Otherwise I think this is good to go.

@ryanplusplus ryanplusplus force-pushed the IgnoreImgDirectory branch 3 times, most recently from e8e3cdd to 9dec9f6 Compare June 16, 2016 01:06
@kytrinyx
Copy link
Member

You're getting failures on master that we're not getting on travis? Could you paste the error?

@kytrinyx kytrinyx merged commit 8d8c198 into exercism:master Jun 16, 2016
@kytrinyx
Copy link
Member

I pulled this down to my machine and it's passing there. I'm still curious what the failure is, if you're getting one—I'd hate to have something that has failures on some systems and not others (that's a kind of terrible experience for contributors).

@ryanplusplus ryanplusplus deleted the IgnoreImgDirectory branch June 16, 2016 12:37
@ryanplusplus
Copy link
Member Author

I'm not on my development machine right now so I can't recreate the failure exactly, but the gist was that instead of getting this from evaluate_test.go:

    // Output:
    // -> No directory found for [crystal].
    // -> config.json does not include [garnet].
    // -> missing example solution in [beryl melanite].
    // -> [diamond] should not be implemented.
    // -> [amethyst beryl crystal] found in multiple categories.

it got:

    // Output:
    // -> No directory found for [crystal].
    // -> config.json does not include [.hidden garnet].
    // -> missing example solution in [beryl melanite].
    // -> [diamond] should not be implemented.
    // -> [amethyst beryl crystal] found in multiple categories.

I tried to debug the issue to figure out why dot/hidden directories were not ignored in the evaluate tests but were successfully ignored in the track tests. Alas, I do not have the tools or skills required to determine what was going on. If I could get fmt.Println to work during the evaluate tests I think I'd have a fighting chance, but I could only ever get printed output during the track tests.

@ryanplusplus
Copy link
Member Author

Also, I got the failing test on Ubuntu 16.04 in case that helps.

@ryanplusplus
Copy link
Member Author

Here's the full output:

~/go/github.com/exercism/configlet::master$ go test ./...
--- FAIL: ExampleEvaluate (0.00s)
got:
-> No directory found for [crystal].
-> config.json does not include [.hidden garnet].
-> missing example solution in [beryl melanite].
-> [diamond] should not be implemented.
-> [amethyst beryl crystal] found in multiple categories.
want:
-> No directory found for [crystal].
-> config.json does not include [garnet].
-> missing example solution in [beryl melanite].
-> [diamond] should not be implemented.
-> [amethyst beryl crystal] found in multiple categories.
FAIL
FAIL    _/home/ryan/go/github.com/exercism/configlet    0.004s
ok      _/home/ryan/go/github.com/exercism/configlet/configlet  0.004s

I re-ran this with the current upstream master and got the same results.

@kytrinyx
Copy link
Member

kytrinyx commented Jun 22, 2016

I wonder if the dot character is specified differently on different platforms. Would you try running the tests against this branch? https://github.com/exercism/configlet/tree/hidden-rune I tried changing the implementation of the hidden directory detection.

@ryanplusplus
Copy link
Member Author

ryanplusplus commented Jun 23, 2016

Looks like the same results:

~/go/github.com/exercism/configlet$ go test ./...
--- FAIL: ExampleEvaluate (0.00s)
got:
-> No directory found for [crystal].
-> config.json does not include [.hidden garnet].
-> missing example solution in [beryl melanite].
-> [diamond] should not be implemented.
-> [amethyst beryl crystal] found in multiple categories.
want:
-> No directory found for [crystal].
-> config.json does not include [garnet].
-> missing example solution in [beryl melanite].
-> [diamond] should not be implemented.
-> [amethyst beryl crystal] found in multiple categories.
FAIL
FAIL    _/home/ryan/go/github.com/exercism/configlet    0.010s
ok      _/home/ryan/go/github.com/exercism/configlet/configlet  0.008s

I am running the tests correctly, right?

@kytrinyx
Copy link
Member

I am running the tests correctly, right?

Yeah. I wish I could reproduce this!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants