Skip to content
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

fix(gas-report): update filename matcher #2277

Merged
merged 4 commits into from
Feb 20, 2024

Conversation

pwrstudio
Copy link
Contributor

Fixes #2276

Copy link

changeset-bot bot commented Feb 18, 2024

🦋 Changeset detected

Latest commit: c1d690b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
@latticexyz/gas-report Patch
@latticexyz/cli Patch
@latticexyz/schema-type Patch
@latticexyz/store Patch
@latticexyz/world-modules Patch
@latticexyz/world Patch
@latticexyz/abi-ts Patch
@latticexyz/block-logs-stream Patch
@latticexyz/common Patch
@latticexyz/config Patch
create-mud Patch
@latticexyz/dev-tools Patch
@latticexyz/ecs-browser Patch
@latticexyz/faucet Patch
@latticexyz/network Patch
@latticexyz/noise Patch
@latticexyz/phaserx Patch
@latticexyz/protocol-parser Patch
@latticexyz/react Patch
@latticexyz/recs Patch
@latticexyz/services Patch
@latticexyz/solecs Patch
solhint-config-mud Patch
solhint-plugin-mud Patch
@latticexyz/std-client Patch
@latticexyz/std-contracts Patch
@latticexyz/store-cache Patch
@latticexyz/store-indexer Patch
@latticexyz/store-sync Patch
@latticexyz/utils Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -130,7 +130,7 @@ async function runGasReport(options: Options): Promise<GasReport> {
const lines = logs.split("\n").map(stripAnsi);
const gasReportPattern = /^\s*GAS REPORT: (\d+) (.*)$/;
const testFunctionPattern = /^\[(?:PASS|FAIL).*\] (\w+)\(\)/;
const testFilePattern = /^Running \d+ tests? for (.*):(.*)$/;
const testFilePattern = /^Ran \d+ tests? for (.*):(.*)$/;
Copy link
Member

Choose a reason for hiding this comment

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

nice catch! thoughts on testing for both instead of just one? so we can support older foundry versions for a little while? maybe with a comment explaining

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! I have amended the commit.

@pwrstudio pwrstudio force-pushed the gas-report-test-filename branch from 8ba9947 to 5a2f702 Compare February 19, 2024 09:54
@holic holic changed the title fix: gas report fails to find test filename fix(gas-report): update filename matcher Feb 20, 2024
@holic
Copy link
Member

holic commented Feb 20, 2024

can you enable us to edit the branch/PR? I'd like to add a changeset so we can merge this

@pwrstudio
Copy link
Contributor Author

Hmm seems to be a bit problematic because my branch is under a org account: https://github.com/orgs/community/discussions/5634
I could make a new PR from a fork on my personal account...
Sorry for the headache over such a small fix.

@holic
Copy link
Member

holic commented Feb 20, 2024

that's okay! another option is to add the changeset yourself

pnpm run changeset

from the root of the mud repo

or click the link in #2277 (comment)

@holic holic merged commit a02da55 into latticexyz:main Feb 20, 2024
10 of 11 checks passed
@holic
Copy link
Member

holic commented Feb 20, 2024

thank you!

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.

Gas report fails to find test filename
2 participants