-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: multiple skips #148
feat: multiple skips #148
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
name: Test Kubo Skipped (e2e) | ||
|
||
on: | ||
workflow_dispatch: | ||
push: | ||
branches: | ||
- main | ||
pull_request: | ||
|
||
jobs: | ||
test: | ||
runs-on: 'ubuntu-latest' | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
target: ['latest', 'master'] | ||
defaults: | ||
run: | ||
shell: bash | ||
steps: | ||
- name: Setup Go | ||
uses: actions/setup-go@v3 | ||
with: | ||
go-version: 1.20.4 | ||
- uses: actions/checkout@v3 | ||
with: | ||
path: 'gateway-conformance' | ||
- name: Extract fixtures | ||
uses: ./gateway-conformance/.github/actions/extract-fixtures | ||
with: | ||
output: fixtures | ||
- uses: protocol/cache-go-action@v1 | ||
- run: go install github.com/ipfs/kubo/cmd/ipfs@${{ matrix.target }} | ||
shell: bash | ||
env: | ||
GOPROXY: direct | ||
- name: Configure Kubo Gateway | ||
run: | | ||
ipfs init; | ||
source ./gateway-conformance/kubo-config.example.sh "$(pwd)/fixtures" | ||
echo "IPFS_NS_MAP=${IPFS_NS_MAP}" >> $GITHUB_ENV | ||
# note: the IPFS_NS_MAP set above will be passed the daemon | ||
- uses: ipfs/start-ipfs-daemon-action@v1 | ||
with: | ||
args: '--offline' | ||
wait-for-addrs: false | ||
- name: Provision Kubo Gateway | ||
run: | | ||
# Import car files | ||
cars=$(find ./fixtures -name '*.car') | ||
for car in $cars | ||
do | ||
ipfs dag import --pin-roots=false --stats "$car" | ||
done | ||
|
||
# Import ipns records | ||
records=$(find ./fixtures -name '*.ipns-record') | ||
for record in $records | ||
do | ||
key=$(basename -s .ipns-record "$record" | cut -d'_' -f1) | ||
ipfs routing put --allow-offline "/ipns/$key" "$record" | ||
done | ||
- name: Run the tests | ||
uses: ./gateway-conformance/.github/actions/test | ||
with: | ||
gateway-url: http://127.0.0.1:8080 | ||
subdomain-url: http://example.com | ||
json: output.json | ||
xml: output.xml | ||
html: output.html | ||
markdown: output.md | ||
skips: '["TestTrustlessCarPathing", "TestNativeDag/GET_plain_JSON_codec_from_.*"]' | ||
- name: Set summary | ||
if: (failure() || success()) | ||
run: cat ./output.md >> $GITHUB_STEP_SUMMARY | ||
- name: Upload one-page HTML report | ||
if: (failure() || success()) | ||
uses: actions/upload-artifact@v3 | ||
with: | ||
name: conformance-${{ matrix.target }}.html | ||
path: ./output.html |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -37,6 +37,7 @@ The `test` command is the main command of the tool. It is used to test a given I | |||||
| html | GitHub Action | The path where the one-page HTML test report should be generated. | `./report.html` | | ||||||
| markdown | GitHub Action | The path where the summary Markdown test report should be generated. | `./report.md` | | ||||||
| specs | Both | A comma-separated list of specs to be tested. Accepts a spec (test only this spec), a +spec (test also this immature spec), or a -spec (do not test this mature spec). | Mature specs only | | ||||||
| skip | Both | Run only the those tests that do not match the regular expression. Similar to golang's skip, the expression is split by slash (/) into a sequence and each part must match the corresponding part in the test name, if any | empty | | ||||||
| args | Both | [DANGER] The `args` input allows you to pass custom, free-text arguments directly to the Go test command that the tool employs to execute tests. | N/A | | ||||||
|
||||||
##### Specs | ||||||
|
@@ -79,6 +80,7 @@ A few examples: | |||||
markdown: report.md | ||||||
html: report.html | ||||||
args: -timeout 30m | ||||||
skips: '["TestGatewaySubdomains", "TestGatewayCar/GET_response_for_application/.*/Header_Content-Length"]' | ||||||
``` | ||||||
|
||||||
##### Docker | ||||||
|
@@ -235,10 +237,11 @@ gateway-conformance test --specs trustless-gateway,-trustless-gateway-ipns | |||||
|
||||||
### Skip a specific test | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we add Include this note:
|
||||||
|
||||||
Tests are skipped using Go's standard syntax: | ||||||
Tests are skipped using the `--skip` parameter and Go's standard syntax: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
```bash | ||||||
gateway-conformance test -skip 'TestGatewayCar/GET_response_for_application/vnd.ipld.car/Header_Content-Length' | ||||||
gateway-conformance test --skip 'TestGatewayCar/GET_response_for_application/vnd.ipld.car/Header_Content-Length' | ||||||
gateway-conformance test --skip 'TestGatewayCar/.*/vnd.ipld.car' --skip 'TestGatewayCar/GET_response_for_application/vnd.*' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. highlight: use in CLI |
||||||
``` | ||||||
|
||||||
### Extracting the test fixtures | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package main | |
|
||
import ( | ||
"bytes" | ||
"encoding/json" | ||
"fmt" | ||
"io" | ||
"log" | ||
|
@@ -76,6 +77,7 @@ func main() { | |
var directory string | ||
var merged bool | ||
var verbose bool | ||
var skips cli.StringSlice | ||
|
||
app := &cli.App{ | ||
Name: "gateway-conformance", | ||
|
@@ -113,6 +115,13 @@ func main() { | |
Value: "", | ||
Destination: &specs, | ||
}, | ||
&cli.StringSliceFlag{ | ||
Name: "skip", | ||
Usage: "Accepts a test path to skip.\nCan be used multiple times.\n" + | ||
"Example: --skip \"TestTar/GET_TAR_with_format=.*/Body\" --skip \"TestGatewayBlock\" --skip \"TestTrustlessCarPathing\"\n" + | ||
"It uses the same syntax as the -skip flag of go test.", | ||
Destination: &skips, | ||
}, | ||
Comment on lines
+118
to
+124
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Parity with |
||
&cli.BoolFlag{ | ||
Name: "verbose", | ||
Usage: "Prints all the output to the console.", | ||
|
@@ -134,10 +143,17 @@ func main() { | |
|
||
fmt.Println("go " + strings.Join(args, " ")) | ||
|
||
// json encode the string array for parameter | ||
skipsList := skips.Value() | ||
skipsJSON, err := json.Marshal(skipsList) | ||
if err != nil { | ||
return fmt.Errorf("failed to marshal skips: %w", err) | ||
} | ||
|
||
output := &bytes.Buffer{} | ||
cmd := exec.Command("go", args...) | ||
cmd.Dir = tooling.Home() | ||
cmd.Env = append(os.Environ(), fmt.Sprintf("GATEWAY_URL=%s", gatewayURL)) | ||
cmd.Env = append(os.Environ(), fmt.Sprintf("GATEWAY_URL=%s", gatewayURL), fmt.Sprintf("TEST_SKIPS=%s", skipsJSON)) | ||
|
||
if subdomainGatewayURL != "" { | ||
cmd.Env = append(cmd.Env, fmt.Sprintf("SUBDOMAIN_GATEWAY_URL=%s", subdomainGatewayURL)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
package test | ||
|
||
import ( | ||
"encoding/json" | ||
"os" | ||
"regexp" | ||
"testing" | ||
) | ||
|
||
const SKIPS_ENV_VAR = "TEST_SKIPS" | ||
|
||
var skips = []string{} | ||
|
||
func GetSkips() []string { | ||
skips := os.Getenv(SKIPS_ENV_VAR) | ||
if skips == "" { | ||
return []string{} | ||
} | ||
|
||
var skipsList []string | ||
err := json.Unmarshal([]byte(skips), &skipsList) | ||
if err != nil { | ||
panic(err) | ||
} | ||
|
||
return skipsList | ||
} | ||
|
||
func split(name string) []string { | ||
// split name by "/" not prefixed with "\" | ||
parts := []string{} | ||
current := "" | ||
hasSlash := false | ||
|
||
for _, c := range name { | ||
if hasSlash { | ||
if c == '/' || c == '\\' { | ||
current += string(c) | ||
hasSlash = false | ||
} else { | ||
current += "\\" | ||
current += string(c) | ||
hasSlash = false | ||
} | ||
continue | ||
} | ||
|
||
if c == '/' { | ||
parts = append(parts, current) | ||
current = "" | ||
continue | ||
} | ||
if c == '\\' { | ||
hasSlash = true | ||
continue | ||
} | ||
|
||
current += string(c) | ||
} | ||
|
||
if current != "" { | ||
parts = append(parts, current) | ||
} | ||
|
||
return parts | ||
} | ||
|
||
func isSkipped(name string, skips []string) bool { | ||
for _, skip := range skips { | ||
if name == skip { | ||
return true | ||
} | ||
|
||
skipParts := split(skip) | ||
nameParts := split(name) | ||
|
||
if len(skipParts) > len(nameParts) { | ||
continue | ||
} | ||
|
||
matches := true | ||
for i := range skipParts { | ||
skipPart := skipParts[i] | ||
skipPart = "^" + skipPart + "$" | ||
|
||
matched, err := regexp.MatchString(skipPart, nameParts[i]) | ||
if err != nil { | ||
panic(err) | ||
} | ||
|
||
if !matched { | ||
matches = false | ||
break | ||
} | ||
} | ||
|
||
return matches | ||
} | ||
|
||
return false | ||
} | ||
|
||
func MustNotBeSkipped(t *testing.T) { | ||
t.Helper() | ||
skipped := isSkipped(t.Name(), skips) | ||
|
||
if skipped { | ||
t.Skipf("skipped") | ||
} | ||
} | ||
|
||
// init will load the skips | ||
func init() { | ||
skips = GetSkips() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Highlight: this is how we skip tests. Note the regexp similar to golang's matching
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the same as the following, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regex support is pretty recent, i think? https://tip.golang.org/doc/go1.20 states:
go help testflag
conforms we now have parity for-run
and-skip
which is really nice!:Both
skip
andrun
are useful.run
allows more fine-grained control than our predefined presets, which saves time when someone develops a fix and has to re-run specific subset of tests without running the entire suite (not a problem today, but the suite will grow).@laurentsenta do we need to have dedicated input in github action?
If we could keep this under
args
we would not have to do anything special to support-run
here, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also wondering if we even need dedicated run/skip gateway conformance cli flags if we can accomplish the same using the go flags.
If we do, e.g. as nicer to use sugar, I think it might be worth considering converting the custom flag values into regex and passing it to go to avoid having to reimplement run/skip logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks for the reviews and comments,
That work might be a dead end, Sorry for the noise 🤦
I ran more experiments, and not everything makes sense to me, but regular skips should be enough.
@galargh your suggestion would need a different parenthesizing:
@lidel your issue should be solved with an
|
and parenthesis:Probably also:
We can drop that PR.
I think the easiest way to think about golang
-skip
is:(g1)|(g2)
at the top level,/
and each part is matched with the go test name.willSkip("regexp1/regexp2", "testX/testY") = match(regexp1, testX) && match(regexp2, testY)
I'm still confused by some aspects of the matching... if you want to dig deeper: https://gist.github.com/laurentsenta/5ed9fe9954605eefc1ed9ee2db7f66a9
But I'll close this PR for now and continue the discussion in #141.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for looking into it 🙇 That's a nice write-up for a tricky feature 👏