diff --git a/foundry/script/DeployPlatform.s.sol b/foundry/script/DeployPlatform.s.sol index a2cbbef..20163b4 100644 --- a/foundry/script/DeployPlatform.s.sol +++ b/foundry/script/DeployPlatform.s.sol @@ -406,7 +406,16 @@ contract DeployPlatform is DeploymentsPath { /// extcodehashes (for `dincli system verify-deployment` check 1 and 3), /// the deployer (ProxyAdmin owner, check 4), and the optional git commit /// hash recorded via GIT_COMMIT_HASH env var (check meta). + /// Writes to this chain's foundry/deployments/.json (DeploymentsPath). function _writeDeployments(Deployment memory d, address deployer) internal { + _writeDeploymentsTo(d, deployer, _deploymentsFile("")); + } + + /// @dev _writeDeployments with an explicit output path. Tests call this with + /// their own file instead of setting DEPLOYMENTS_NETWORK: vm.setEnv + /// changes the environment shared by forge's parallel test contracts, + /// so two tests setting it to different values race (PR No. 254). + function _writeDeploymentsTo(Deployment memory d, address deployer, string memory outPath) internal { // Resolve implementations via EIP-1967 slot reads. address implTreasury = Upgrades.getImplementationAddress(d.dinTreasury); address implToken = Upgrades.getImplementationAddress(d.dinToken); @@ -461,7 +470,6 @@ contract DeployPlatform is DeploymentsPath { string memory outDir = string.concat(vm.projectRoot(), "/deployments"); vm.createDir(outDir, true); - string memory outPath = _deploymentsFile(""); vm.writeJson(finalJson, outPath); console.log("Deployments written to:", outPath); } diff --git a/foundry/test/DeployPlatformScript.t.sol b/foundry/test/DeployPlatformScript.t.sol index ce8aac9..56cc218 100644 --- a/foundry/test/DeployPlatformScript.t.sol +++ b/foundry/test/DeployPlatformScript.t.sol @@ -8,10 +8,10 @@ import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/Own import {DeployPlatform} from "../script/DeployPlatform.s.sol"; -/// @dev Exposes the internal _writeDeployments for white-box testing. +/// @dev Exposes the internal _writeDeploymentsTo for white-box testing. contract DeployPlatformHarness is DeployPlatform { - function writeDeployments(Deployment memory d, address deployer) external { - _writeDeployments(d, deployer); + function writeDeploymentsTo(Deployment memory d, address deployer, string memory outPath) external { + _writeDeploymentsTo(d, deployer, outPath); } } import {DinCoordinator} from "../src/DinCoordinator.sol"; @@ -28,10 +28,10 @@ import {DinEmission} from "../src/DinEmission.sol"; /// environment, so no deploy test depends on env. The env-read path /// (readTokenomics) is covered by exactly one test, which is the only /// place in foundry/test that sets these tokenomics keys; see its comment. -/// test_writeDeployments_writesImplAndCodehashMetadata calls _writeDeployments -/// via DeployPlatformHarness using DEPLOYMENTS_NETWORK=test-impl-metadata, -/// so it writes foundry/deployments/test-impl-metadata.json rather than -/// clobbering foundry/deployments/localhost.json. Run with: +/// test_writeDeployments_writesImplAndCodehashMetadata calls +/// _writeDeploymentsTo via DeployPlatformHarness with an explicit path, +/// foundry/deployments/test-impl-metadata.json, so it never clobbers +/// localhost.json and never sets DEPLOYMENTS_NETWORK. Run with: /// forge test --match-contract DeployPlatformScriptTest -vv contract DeployPlatformScriptTest is Test { DeployPlatform internal script; @@ -169,22 +169,19 @@ contract DeployPlatformScriptTest is Test { } /// @dev Verifies the JSON-writing cycle used by dincli system verify-deployment. - /// Calls _writeDeployments via DeployPlatformHarness (which exposes the - /// internal function) with a DEPLOYMENTS_NETWORK override to avoid - /// clobbering foundry/deployments/localhost.json. - /// - /// vm.setEnv cannot unset a key, so DEPLOYMENTS_NETWORK stays set for the - /// rest of the run. Any future test that reads DEPLOYMENTS_NETWORK MUST - /// go into this function, or it will see "test-impl-metadata". + /// Calls _writeDeploymentsTo via DeployPlatformHarness with this test's + /// own file. It deliberately doesn't set DEPLOYMENTS_NETWORK: the + /// process environment is shared by forge's parallel test contracts, and + /// UpgradePlatformScriptTest setting it to another file name made this + /// test read a file that was never written (PR No. 254). function test_writeDeployments_writesImplAndCodehashMetadata() public { - vm.setEnv("DEPLOYMENTS_NETWORK", "test-impl-metadata"); DeployPlatform.Tokenomics memory t = script.defaultTokenomics(); DeployPlatform.Deployment memory d = script.deploy(t, deployer); - harness.writeDeployments(d, deployer); string memory path = string.concat( vm.projectRoot(), "/deployments/test-impl-metadata.json" ); + harness.writeDeploymentsTo(d, deployer, path); string memory content = vm.readFile(path); address implTreasury = vm.parseJsonAddress(content, ".dinTreasuryImpl"); diff --git a/foundry/test/UpgradePlatformScript.t.sol b/foundry/test/UpgradePlatformScript.t.sol index db40e13..9112db2 100644 --- a/foundry/test/UpgradePlatformScript.t.sol +++ b/foundry/test/UpgradePlatformScript.t.sol @@ -19,11 +19,11 @@ contract UpgradePlatformHarness is UpgradePlatform { } } -/// @dev Exposes _writeDeployments to create the initial deployments JSON needed +/// @dev Exposes _writeDeploymentsTo to create the initial deployments JSON needed /// by the upgrade metadata test, without coupling to DeployPlatformScript.t.sol. contract DeployHarness is DeployPlatform { - function writeDeployments(Deployment memory d, address deployer_) external { - _writeDeployments(d, deployer_); + function writeDeploymentsTo(Deployment memory d, address deployer_, string memory outPath) external { + _writeDeploymentsTo(d, deployer_, outPath); } } @@ -32,10 +32,9 @@ contract DeployHarness is DeployPlatform { /// deployments JSON records the correct implementation address, /// extcodehash, and (when GIT_COMMIT_HASH is supplied) commit hash. /// -/// test_writeUpgradeMetadata_writesImplCodehashAndCommit sets -/// DEPLOYMENTS_NETWORK=test-upgrade-metadata, which stays set for the -/// entire forge process. Any future test in this contract that reads -/// DEPLOYMENTS_NETWORK MUST go into that function. +/// The test passes its own file, foundry/deployments/test-upgrade-metadata.json, +/// explicitly and sets no environment variables: vm.setEnv changes the +/// environment shared by forge's parallel test contracts (PR No. 254). /// Run with: forge test --match-contract UpgradePlatformScriptTest -vv contract UpgradePlatformScriptTest is Test { UpgradePlatformHarness internal harness; @@ -53,23 +52,16 @@ contract UpgradePlatformScriptTest is Test { /// (a) recorded implementation address matches the upgraded impl, /// (b) recorded codehash matches extcodehash(newImpl), /// (c) .deployedAtCommit is written when a non-empty commit is passed. - /// - /// Both assertions live in one function because vm.setEnv cannot unset - /// a key, so DEPLOYMENTS_NETWORK stays as "test-upgrade-metadata" and - /// GIT_COMMIT_HASH stays set for the rest of the run. Any future test - /// that reads either key MUST go into this function. function test_writeUpgradeMetadata_writesImplCodehashAndCommit() public { - vm.setEnv("DEPLOYMENTS_NETWORK", "test-upgrade-metadata"); + string memory path = string.concat( + vm.projectRoot(), "/deployments/test-upgrade-metadata.json" + ); // Write an initial deployments JSON so the three-arg vm.writeJson calls // inside _writeUpgradeMetadata have an existing file to update. DeployPlatform.Tokenomics memory t = deployScript.defaultTokenomics(); DeployPlatform.Deployment memory d = deployScript.deploy(t, deployer); - deployHarness.writeDeployments(d, deployer); - - string memory path = string.concat( - vm.projectRoot(), "/deployments/test-upgrade-metadata.json" - ); + deployHarness.writeDeploymentsTo(d, deployer, path); // Deploy DinTokenV2 to get a real implementation address and codehash. // We only need the bytecode to be present; the contract need not be @@ -91,11 +83,10 @@ contract UpgradePlatformScriptTest is Test { // --- (c): commit hash written when non-empty --- string memory commit = "abc1234def5678900000000000000000000000000"; - vm.setEnv("GIT_COMMIT_HASH", commit); harness.writeUpgradeMetadata(newImpl, "dinToken", path, commit); content = vm.readFile(path); string memory recordedCommit = vm.parseJsonString(content, ".deployedAtCommit"); - assertEq(recordedCommit, commit, ".deployedAtCommit must match GIT_COMMIT_HASH"); + assertEq(recordedCommit, commit, ".deployedAtCommit must match the commit passed in"); } }