Skip to content

Conversation

@wpfleger96
Copy link
Collaborator

@wpfleger96 wpfleger96 commented Oct 16, 2025

Summary

Fixes race condition causing test_pricing_cache_performance test to fail intermittently in CI with:

"No such file or directory (os error 2)"

Currently all 4 pricing integration tests share the same GOOSE_CACHE_DIR environment variable. Since environment variables are process-wide, when tests run in parallel (cargo test --jobs 2 in CI), one test can overwrite another's env var value. This creates a race window where Test A sets the env var, Test B overwrites it, then Test A reads Test B's (potentially deleted) path and fails. This is resulting in test failures on my unrelated PR

#3546 previously added TempDir isolation, but this only provided per-test directories and didn't prevent the tests from running in parallel and overwriting each other's environment variables.

To address this I added #[serial] attributes to all 4 pricing integration tests. This forces sequential execution of these tests only, while other tests continue running in parallel.

UPDATE: per feedback I'm just going to delete the flaky tests

Type of Change

  • Feature
  • Bug fix
  • Refactor / Code quality
  • Performance improvement
  • Documentation
  • Tests
  • Security fix
  • Build / Release
  • Other (specify below)

Testing

I had trouble reproducing this locally, so I created a synthetic test (race_condition_demo.rs) locally that deliberately triggers the race condition:

  #[tokio::test]
  async fn race_test_1() {
      let temp_dir = TempDir::new().unwrap();
      std::env::set_var("GOOSE_CACHE_DIR", temp_dir.path());
      sleep(Duration::from_millis(50)).await;  // Widen race window

      let read_path = std::env::var("GOOSE_CACHE_DIR").unwrap();
      // Fails if Test 2 overwrote the env var
      assert!(Path::new(&read_path).exists());
  }

  #[tokio::test]
  async fn race_test_2() {
      sleep(Duration::from_millis(10)).await;  // Let Test 1 start first
      let temp_dir = TempDir::new().unwrap();
      std::env::set_var("GOOSE_CACHE_DIR", temp_dir.path());  // Overwrites Test 1!
      drop(temp_dir);  // Delete directory Test 1 might try to use
  }

Results:

  • Without #[serial]: Test 1 panics with RACE CONDITION! Path doesn't exist: /tmp/.tmppTBwjl (Test 2's deleted path)
  • With #[serial]: Both tests pass—Test 1 reads its own path correctly

Additional validation: Stress tested the fixed pricing tests with 10 consecutive runs using --jobs 4. All runs passed.

@wpfleger96 wpfleger96 marked this pull request as ready for review October 16, 2025 17:31
@wpfleger96 wpfleger96 changed the title fix race condition in pricing integration tests delete flaky pricing integration tests Oct 17, 2025
@DOsinga DOsinga merged commit bcd3a0d into main Oct 17, 2025
11 checks passed
@DOsinga DOsinga deleted the wpfleger/fix-pricing-tests branch October 17, 2025 19:58
wpfleger96 added a commit to wpfleger96/goose that referenced this pull request Oct 17, 2025
* main:
  delete flaky pricing integration tests (block#5207)
  chore: upgrade most npm packages to latest (block#5185)
  Release/1.11.0 (block#5224)
  Rewrite extension management tools (block#5057)
  fix: re-sync package-lock.json (block#5235)
  docs: Hacktoberfest MCP youtube short entry to community-content.json (block#5150)
  feat: add schedule button to recipe entries (block#5217)
  Autocompact threshold UI cleanup (block#5232)
  fix: correct schema for openai tools (block#5229)
  Break compaction back into check_ and do_ compaction (block#5212)
  fix: revert built app name to uppercase Goose (block#5206)
  feat: add Code Documentation Generator recipe (block#5121) (block#5125)
michaelneale added a commit that referenced this pull request Oct 18, 2025
* main:
  Standardize Session Name Attribute (#5085)
  Docs: Include step-by-step model configuration instructions for first… (#5239)
  Delete message visibility filter (#5216)
  delete flaky pricing integration tests (#5207)
  chore: upgrade most npm packages to latest (#5185)
  Release/1.11.0 (#5224)
  Rewrite extension management tools (#5057)
  fix: re-sync package-lock.json (#5235)
  docs: Hacktoberfest MCP youtube short entry to community-content.json (#5150)
  feat: add schedule button to recipe entries (#5217)
  Autocompact threshold UI cleanup (#5232)
  fix: correct schema for openai tools (#5229)
tlongwell-block added a commit that referenced this pull request Oct 18, 2025
* origin/main: (66 commits)
  Revert "Rewrite extension management tools" (#5243)
  Standardize Session Name Attribute (#5085)
  Docs: Include step-by-step model configuration instructions for first… (#5239)
  Delete message visibility filter (#5216)
  delete flaky pricing integration tests (#5207)
  chore: upgrade most npm packages to latest (#5185)
  Release/1.11.0 (#5224)
  Rewrite extension management tools (#5057)
  fix: re-sync package-lock.json (#5235)
  docs: Hacktoberfest MCP youtube short entry to community-content.json (#5150)
  feat: add schedule button to recipe entries (#5217)
  Autocompact threshold UI cleanup (#5232)
  fix: correct schema for openai tools (#5229)
  Break compaction back into check_ and do_ compaction (#5212)
  fix: revert built app name to uppercase Goose (#5206)
  feat: add Code Documentation Generator recipe (#5121) (#5125)
  Revert "feat: enhance goose to search sessions for easy recall (#5177)" (#5209)
  Blog: Best Practices for Prompt Engineering with goose (#5204)
  force WAL sync after session create (#5202)
  Feat: goose Apify MCP integration docs (#5047)
  ...
lifeizhou-ap added a commit that referenced this pull request Oct 19, 2025
* main: (32 commits)
  turn off WAL (#5203)
  Skip subagents for gemini (#5257)
  Revert "Standardize Session Name Attribute" (#5250)
  improve provider request logging a bit (#5236)
  Fix OpenAI empty choices panic (#5248)
  Revert "Rewrite extension management tools" (#5243)
  Standardize Session Name Attribute (#5085)
  Docs: Include step-by-step model configuration instructions for first… (#5239)
  Delete message visibility filter (#5216)
  delete flaky pricing integration tests (#5207)
  chore: upgrade most npm packages to latest (#5185)
  Release/1.11.0 (#5224)
  Rewrite extension management tools (#5057)
  fix: re-sync package-lock.json (#5235)
  docs: Hacktoberfest MCP youtube short entry to community-content.json (#5150)
  feat: add schedule button to recipe entries (#5217)
  Autocompact threshold UI cleanup (#5232)
  fix: correct schema for openai tools (#5229)
  Break compaction back into check_ and do_ compaction (#5212)
  fix: revert built app name to uppercase Goose (#5206)
  ...
katzdave added a commit that referenced this pull request Oct 20, 2025
* 'main' of github.com:block/goose:
  docs: provide more clarity in tagging step of releases (#5269)
  docs: add GOOSE_DEBUG environment variable (#5265)
  Lifei/UI parameter input (#5222)
  turn off WAL (#5203)
  Skip subagents for gemini (#5257)
  Revert "Standardize Session Name Attribute" (#5250)
  improve provider request logging a bit (#5236)
  Fix OpenAI empty choices panic (#5248)
  Revert "Rewrite extension management tools" (#5243)
  Standardize Session Name Attribute (#5085)
  Docs: Include step-by-step model configuration instructions for first… (#5239)
  Delete message visibility filter (#5216)
  delete flaky pricing integration tests (#5207)
  chore: upgrade most npm packages to latest (#5185)
  Release/1.11.0 (#5224)
michaelneale added a commit that referenced this pull request Oct 21, 2025
* main: (40 commits)
  Fix extension manager resource deadlock (#5066)
  add new prompt for memory extension (#4945)
  feat: Stable system prompt and tool ordering for more cache hits (#5192)
  Remove gtag analytics error (#5268)
  Feat/smart task organizer recipe (#5032)
  docs: `goose recipe open` command (#5264)
  docs: provide more clarity in tagging step of releases (#5269)
  docs: add GOOSE_DEBUG environment variable (#5265)
  Lifei/UI parameter input (#5222)
  turn off WAL (#5203)
  Skip subagents for gemini (#5257)
  Revert "Standardize Session Name Attribute" (#5250)
  improve provider request logging a bit (#5236)
  Fix OpenAI empty choices panic (#5248)
  Revert "Rewrite extension management tools" (#5243)
  Standardize Session Name Attribute (#5085)
  Docs: Include step-by-step model configuration instructions for first… (#5239)
  Delete message visibility filter (#5216)
  delete flaky pricing integration tests (#5207)
  chore: upgrade most npm packages to latest (#5185)
  ...
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.

3 participants