Add list command #1113
Add list command #1113
Conversation
|
Hello. Thanks for opening a PR on Exercism. We are currently in a phase of our journey where we have paused community contributions to allow us to take a breather and redesign our community model. You can learn more in this blog post. As such, all issues and PRs in this repository are being automatically closed. That doesn't mean we're not interested in your ideas, or that if you're stuck on something we don't want to help. The best place to discuss things is with our community on the Exercism Community Forum. You can use this link to copy this into a new topic there. Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it. |
There was a problem hiding this comment.
Thanks for your work on this. I've reopened the PR :)
I have three general comments:
- I really like the list functionality. I'm 👍 on adding that.
- I don't love the download all functionality, as that has the downside of marking all those exercises in progress. I'd like to have a wider community discussion about that first, and also take time to consider it from a product perspective.
- There are quite a few other changes in this PR that make it hard to see exactly what's going on where. I've added questions to those for clarification.
Action suggestions:
- I think there are three seperate PRs here.
a) The refactorings
b) The addition of List
c) The addition of Download
I'd like to see a PR just for (b) and potentially one for (a) to start with. Those will be more manageable to review. In its current state, there's so much happening in this PR, I think it will take a long time to get a proper review. I'd like to get the List stuff reviewed properly instead and be ale to deploy that, without waiting on the rest to get reviewesd.
The List functionality also needs a comprehensive test adding.
Thanks for your work on this. I hope that makes sense!
| @@ -116,7 +124,7 @@ func runConfigure(configuration config.Config, flags *pflag.FlagSet) error { | |||
|
|
|||
| // Verify that the token is valid. | |||
| if !skipVerification { | |||
| client, err := api.NewClient(token, baseURL) | |||
| client, err := api.NewClient("", baseURL) | |||
| assert.Regexp(t, "Welcome to Exercism", err.Error()) | ||
| // It uses the default base API url to infer the host | ||
| assert.Regexp(t, "exercism.io/my/settings", err.Error()) | ||
| assert.Regexp(t, "there is no token configured", err.Error()) |
There was a problem hiding this comment.
This and all the other changes you mention below is a poor attempt at "fixing" the broken tests on master. Additionally, I need to revert some of the changes that fix deprecated code, mostly from the io/ioutil to io and os packages.
There was a problem hiding this comment.
As far as I can see, all tests on main are green: https://github.com/exercism/cli/actions/runs/6098825660/job/16549554194
There was a problem hiding this comment.
According to CONTRIBUTING.MD you can run tests locally with go test ./... and this fails on a clean clone and master. I get this:
pwd && go test ./...
/Users/baduker/Downloads/cli
? github.com/exercism/cli/browser [no test files]
? github.com/exercism/cli/exercism [no test files]
ok github.com/exercism/cli/api 0.254s
ok github.com/exercism/cli/cli 0.315s
--- FAIL: TestSubmitFiles (0.01s)
Error Trace: submit_test.go:275
Error: Not equal: "This is file 2." (expected)
!= "" (actual)
--- FAIL: TestSubmitFilesForTeamExercise (0.00s)
Error Trace: submit_test.go:471
Error: Not equal: "This is file 2." (expected)
!= "" (actual)
FAIL
FAIL github.com/exercism/cli/cmd 0.529s
ok github.com/exercism/cli/config 0.370s
ok github.com/exercism/cli/debug 0.424s
ok github.com/exercism/cli/workspace (cached)
FAIL
There was a problem hiding this comment.
Do you mean master or main? The primary branch on this repo is main.
There was a problem hiding this comment.
Yes, I meant main, not master.
There was a problem hiding this comment.
Hmmm, I don't know why that's not working on your machine when it is working on our CI. Maybe a different version of Go? CCing @junedev, @andrerfcsantos, @kytrinyx and @ErikSchierboom to see if they have any ideas :)
There was a problem hiding this comment.
@iHiD @ErikSchierboom the failing tests happen when using a version of Go 1.17 and above. There was a change to the mime/multipart pkg that changes the file path information being returned for any files included in the mocked request. It looks like there's an open PR for this issue #1066 that can be merged in.
There was a problem hiding this comment.
I've replied there. I'd appreciate eyes on that too pls :)
https://github.com/exercism/cli/pull/1066/files#r1331620696
| @@ -40,7 +38,7 @@ func TestDownloadWithoutWorkspace(t *testing.T) { | |||
|
|
|||
| err := runDownload(cfg, pflag.NewFlagSet("fake", pflag.PanicOnError), []string{}) | |||
| if assert.Error(t, err) { | |||
| assert.Regexp(t, "re-run the configure", err.Error()) | |||
| assert.Regexp(t, "need an --exercise name or a solution --uuid", err.Error()) | |||
| @@ -54,7 +52,7 @@ func TestDownloadWithoutBaseURL(t *testing.T) { | |||
|
|
|||
| err := runDownload(cfg, pflag.NewFlagSet("fake", pflag.PanicOnError), []string{}) | |||
| if assert.Error(t, err) { | |||
| assert.Regexp(t, "re-run the configure", err.Error()) | |||
| assert.Regexp(t, "need an --exercise name or a solution --uuid", err.Error()) | |||
| path: tc.file, | ||
| baseURL: "http://www.example.com/", | ||
| path: tc.file, | ||
| sourceURL: "http://www.example.com/", |
|
|
||
| if url != tc.expectedURL { | ||
| t.Fatalf("Expected URL '%s', got '%s'", tc.expectedURL, url) | ||
| } |
|
I'm closing the PR as this is going to be broken down into two (or more) PRs, starting with this one here. |
List all Exercism tracks, exercises implementation, and download selected exercises.
What does this PR do?
The PR implements a feature in the exercism-cli for listing all the available Exercism tracks and exercises based on the given track and difficulty level. It also allows the users to download the list of exercises for a specific track and difficulty level.
Changes Made:
list, to the CLI. This command can take the following flags--track,--all,--level, and--download.downlaod.gocommand for readabilityMakeRequesttoapi.goRelated Issue:
How to test it?
add-list-commandgo build exercism/main.go./main configure --token=YOUR_TOKEN_VALUEStart testing the new command.
Note: the results will be different because the CLI shows tracks and exercises available to a given user.
List your tracks: Run
./main listThis should return:
Note: To see all tracks, run
./main list --all.List all exercises for a given track: Run
./main --list --track=awkThis should return:
Filter exercises for a given track using difficulty: Run
./main list --track=awk --level=hard.This should output:
Download selected exercises for a given track: Run
./main list --track=awk --level=hard --download.This should output: