Skip to content

Conversation

@bluepal-prasanthi-moparthi
Copy link
Collaborator

@bluepal-prasanthi-moparthi bluepal-prasanthi-moparthi commented Dec 1, 2025

… without V8


Note

Guard/skip V8-dependent tests (via requireV8Enabled and version checks), align timeouts to defaultTestTimeout, and make small test/doc robustness updates.

  • Tests (V2):
    • Add requireV8Enabled and apply to V8-dependent tests (foxx, tasks, UDFs, JS transactions, async job pending/delete-expired, admin script, routing table reload).
    • Switch several test timeouts to defaultTestTimeout; add requireClusterMode where needed; relax status check in CallWithChecks to accept 404 or 501.
  • Tests (V1):
    • Introduce skipAboveVersion and use it to skip V8-dependent tests above 3.12.6-1 (async job pending/delete/expired, JS transactions).
  • API/Docs:
    • Clarify ClientAdmin.ReloadRoutingTable comment to mention Foxx routing rebuild.
  • Misc:
    • Minor Makefile comment additions for alternative images.

Written by Cursor Bugbot for commit cb2476a. This will update automatically on new commits. Configure here.

@cla-bot cla-bot bot added the cla-signed label Dec 1, 2025
Copy link

@cursor cursor bot left a 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)
}
Copy link

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.

Fix in Cursor Fix in Web

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
Copy link

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.

Fix in Cursor Fix in Web

c := createClient(t, nil)
skipBelowVersion(c, "3.2", t)
// for disabling v8 tests
skipAboveVersion(c, "3.12.6-1", t)
Copy link

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.

Fix in Cursor Fix in Web

@bluepal-prasanthi-moparthi bluepal-prasanthi-moparthi force-pushed the feature/disable_v8_version_tests branch from d23b95e to cb2476a Compare December 5, 2025 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants