Skip to content

Remove external calls from test fixtures#232

Closed
ChiragAgg5k wants to merge 1 commit into
mainfrom
fix/local-executor-test-fixtures
Closed

Remove external calls from test fixtures#232
ChiragAgg5k wants to merge 1 commit into
mainfrom
fix/local-executor-test-fixtures

Conversation

@ChiragAgg5k
Copy link
Copy Markdown
Member

What changed

  • Replaced DummyJSON calls in executor runtime test fixtures with deterministic local todo data.
  • Removed unused fixture HTTP client dependencies where they only existed for those external calls.

Why

Executor E2E tests should not depend on external network availability. The fixture response shape is unchanged for assertions, including todo.userId = 13.

Testing

  • composer format
  • php -l tests/resources/functions/php/index.php
  • node --check tests/resources/functions/node/index.js
  • python3 -m py_compile tests/resources/functions/python/index.py

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 28, 2026

Greptile Summary

This PR replaces external dummyjson.com HTTP calls in all 8 E2E test fixture runtimes with deterministic hardcoded local todo data, and removes the now-unused HTTP client dependencies from each runtime's manifest. The response shape is preserved, including userId: 13, keeping existing assertions intact.

  • All runtimes (Node, PHP, Python, Deno, Dart, Ruby, .NET, C++): external fetch removed; local todo object with fixed id, todo, completed, and userId fields substituted inline.
  • Dependency manifests updated: node-fetch, guzzlehttp/guzzle, requests, httparty, dio, and libcurl removed from their respective package files.
  • C++ fixture is the only runtime that omits a default value for id before calling stoi, which would throw on an empty string — all other runtimes guard this case.

Confidence Score: 4/5

Safe to merge; the change is narrowly scoped to test fixtures and all runtime assertions remain intact.

The change is straightforward and well-executed across all runtimes. The only inconsistency is in the C++ fixture where stoi(id) is called without a fallback default, unlike every other runtime that guards against a missing id. This would throw if the body omits id, but the current test always supplies it, so no test regression is expected.

tests/resources/functions/cpp/index.cc — the only file missing a default id guard before the stoi call.

Important Files Changed

Filename Overview
tests/resources/functions/cpp/index.cc Removed libcurl HTTP call; replaced with local todo data. stoi(id) has no default guard unlike all other runtimes.
tests/resources/functions/dart/lib/index.dart Removed dio HTTP dependency; local todo map with default id guard via ?? '1'.
tests/resources/functions/dart/pubspec.yaml Removed dio dependency; replaced with dependencies: {}.
tests/resources/functions/deno/index.ts Removed axiod import and external fetch; replaced with local todo object.
tests/resources/functions/dotnet/Index.cs Removed HttpClient and Newtonsoft.Json; local todo dictionary with int.Parse(id) safe due to existing default "1".
tests/resources/functions/node/index.js Removed node-fetch dependency; local todo object with ?? 1 default.
tests/resources/functions/php/index.php Removed Guzzle HTTP client; local todo array with ?? 1 default.
tests/resources/functions/python/index.py Removed requests and json imports; local todo dict with .get('id', 1) default.
tests/resources/functions/ruby/index.rb Removed httparty dependency; local todo hash with `
tests/resources/functions/node/package.json Removed node-fetch from dependencies.
tests/resources/functions/php/composer.json Removed ext-curl and guzzlehttp/guzzle dependencies.
tests/resources/functions/python/requirements.txt Removed requests package, file is now empty.
tests/resources/functions/ruby/Gemfile Removed httparty gem.

Reviews (1): Last reviewed commit: "fix: remove external calls from test fix..." | Re-trigger Greptile

Comment on lines 21 to +24
std::string id = payload["id"].asString();

Json::CharReaderBuilder builder;
Json::CharReader *reader = builder.newCharReader();

CURL *curl;
CURLcode curlRes;
std::string todoBuffer;

curl = curl_easy_init();
if (curl)
{
std::string url = "https://dummyjson.com/todos/" + id;
curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteCallback);
curl_easy_setopt(curl, CURLOPT_WRITEDATA, &todoBuffer);
curlRes = curl_easy_perform(curl);
curl_easy_cleanup(curl);
}

Json::Value todo;
reader->parse(
todoBuffer.c_str(),
todoBuffer.c_str() + todoBuffer.size(),
&todo,
nullptr
);

delete reader;
todo["id"] = stoi(id);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Missing default for id before stoi call

Every other runtime in this PR guards against a missing id in the request body (e.g. Node uses ?? 1, PHP uses ?? 1, .NET defaults to "1", Ruby uses || '1'). The C++ fixture does not: if payload["id"] is absent, asString() returns "" and stoi("") throws std::invalid_argument, crashing the function. The test currently always sends id = "2" so tests still pass, but the fixture diverges from the defensive pattern used in every other language.

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.

1 participant