Skip to content

Add assertFilesMatch helper. NFC#27032

Merged
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:assertFilesMatch
Jun 12, 2026
Merged

Add assertFilesMatch helper. NFC#27032
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:assertFilesMatch

Conversation

@sbc100

@sbc100 sbc100 commented May 29, 2026

Copy link
Copy Markdown
Collaborator

This works just like the assertFileContents but take two filenames. The advantage of this appoach is that the caller does not need to read the file into memory and the filename is not lost (i.e. it can be shown accurately in the diff)

@sbc100 sbc100 requested review from dschuff and kripken May 29, 2026 19:20
Comment thread test/common.py
self.fail(fail_message)

def assertFileContents(self, filename, contents):
def assertFileContents(self, filename, contents, tofile=None):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we didn't reuse assertFileContents, we could just use the builtin filecmp module. This would be simple and maybe faster than reading both files into memory, perhaps at the cost of not being able to print actual diffs. Would that be desirable?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think I prefer to have the existing logic in place for nicely displaying diffs.

I don't think there is any perfermance issues since since we don't have any huge files checked in at all. They all measured in kb.

This works just like the `assertFileContents` but take two filenames.
@sbc100 sbc100 force-pushed the assertFilesMatch branch from 2be9262 to 254d1d3 Compare May 29, 2026 20:17
@sbc100 sbc100 merged commit e5507ba into emscripten-core:main Jun 12, 2026
39 checks passed
@sbc100 sbc100 deleted the assertFilesMatch branch June 12, 2026 21:29
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.

2 participants