-
Notifications
You must be signed in to change notification settings - Fork 81
Disable V8-dependent API tests for V2 since future arangods will ship… #722
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
base: master
Are you sure you want to change the base?
Disable V8-dependent API tests for V2 since future arangods will ship… #722
Conversation
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.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| t.Logf("V8-Version %s", versionInfo.Details["v8-version"]) | ||
| if err != nil { | ||
| t.Fatalf("Failed to get version info with details: %s", err) | ||
| } |
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.
Bug: Error checked after using potentially invalid result
The requireV8Enabled function accesses versionInfo.Details["v8-version"] on line 149 before checking if the VersionWithOptions call returned an error on line 150. If the API call fails, versionInfo may be empty or have nil Details, causing a potential nil map access or logging incorrect information before the error is properly handled and the test is terminated with Fatalf.
| ARANGODB ?= arangodb/enterprise:latest | ||
| STARTER ?= arangodb/arangodb-starter:latest | ||
| # ARANGODB ?= public.ecr.aws/b0b8h2r4/enterprise-preview:2025-12-01-devel-d759089-amd64 | ||
| # STARTER ?= arangodb/arangodb-starter:local-test |
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.
Bug: Test configuration comments may be accidentally committed
Commented-out test configuration lines referencing specific preview images (enterprise-preview:2025-12-01-devel-d759089-amd64) and local-test starter appear to be developer-specific test settings that were likely not intended to be committed. While harmless as comments, they add clutter and may confuse other contributors.
| c := createClient(t, nil) | ||
| skipBelowVersion(c, "3.2", t) | ||
| // for disabling v8 tests | ||
| skipAboveVersion(c, "3.12.6-1", t) |
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.
Bug: Minimum version check accidentally removed from transaction test
The skipBelowVersion(c, "3.2", t) check was removed and replaced with only skipAboveVersion(c, "3.12.6-1", t). The original check ensured the test wouldn't run on ArangoDB versions below 3.2 that may not support JavaScript transactions. Both checks serve different purposes and should coexist: the minimum version check protects against old versions lacking the feature, while the maximum version check skips tests on future versions without V8.
d23b95e to
cb2476a
Compare
… without V8
Note
Guard/skip V8-dependent tests (via requireV8Enabled and version checks), align timeouts to defaultTestTimeout, and make small test/doc robustness updates.
requireV8Enabledand apply to V8-dependent tests (foxx, tasks, UDFs, JS transactions, async job pending/delete-expired, admin script, routing table reload).defaultTestTimeout; addrequireClusterModewhere needed; relax status check inCallWithChecksto accept404or501.skipAboveVersionand use it to skip V8-dependent tests above3.12.6-1(async job pending/delete/expired, JS transactions).ClientAdmin.ReloadRoutingTablecomment to mention Foxx routing rebuild.Written by Cursor Bugbot for commit cb2476a. This will update automatically on new commits. Configure here.