Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix loops in sharness tests to fail the test if the inner command fails #4482

Merged
merged 5 commits into from
Dec 16, 2017

Conversation

Stebalien
Copy link
Member

@Stebalien Stebalien commented Dec 13, 2017

This was hiding errors.

Blocked on: #4453

@ghost ghost assigned Stebalien Dec 13, 2017
@ghost ghost added the status/in-progress In progress label Dec 13, 2017
@@ -171,7 +171,6 @@ var rootROOldSubcommands = map[string]*oldcmds.Command{
"links": ocmd.ObjectLinksCmd,
"get": ocmd.ObjectGetCmd,
"stat": ocmd.ObjectStatCmd,
"patch": ocmd.ObjectPatchCmd,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@Stebalien Stebalien added the status/blocked Unable to be worked further until needs are met label Dec 13, 2017
@@ -139,11 +139,32 @@ test_expect_success "refs IPFS directory file through readonly API succeeds" '
test_curl_gateway_api "refs?arg=$HASH2/test"
'

test_expect_success "test gateway api is sanitized" '
for cmd in "add" "block/put" "bootstrap" "config" "dht" "diag" "dns" "get" "id" "mount" "name/publish" "object/put" "object/new" "object/patch" "pin" "ping" "refs/local" "repo" "resolve" "stats" "swarm" "file" "update" "version" "bitswap"; do
for cmd in add \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed get, version, resolve, and dns as these are now exposed on the gateway.

@whyrusleeping
Copy link
Member

You have angered the CI gods. You must now pay a penance

@Stebalien
Copy link
Member Author

@whyrusleeping that was intentional, these tests are now broken.

@whyrusleeping
Copy link
Member

@Stebalien don't break tests. Thats considered a bad thing.

@whyrusleeping
Copy link
Member

You could probably just delete the tests that are failing.

@Stebalien
Copy link
Member Author

Stebalien commented Dec 14, 2017

@whyrusleeping the tests will be fixed after merging #4453. I opened this so I don't forget to fix them. I could have fixed them there but that PR keeps on changing and getting pushed back so I figured I'd reduce the noise and make a separate PR (blocked on it).

This was hiding errors.

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
This was discussed [here][1] but ignored.

[1]: bfff3fa#r15654625

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien Stebalien removed the status/blocked Unable to be worked further until needs are met label Dec 15, 2017
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
We expect it to return a command error, not a 404, because `local` will be
interpreted as a path.

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien Stebalien added RFM and removed status/in-progress In progress labels Dec 15, 2017
@Stebalien
Copy link
Member Author

Test failure was #4494

@Stebalien Stebalien added need/review Needs a review and removed RFM labels Dec 16, 2017
@whyrusleeping whyrusleeping merged commit 34ade52 into master Dec 16, 2017
@whyrusleeping whyrusleeping deleted the fix/fix-sharness-loops branch December 16, 2017 00:45
@Stebalien Stebalien removed the need/review Needs a review label Dec 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants