Skip to content

Add basic testing for update-in-place flag#1230

Merged
davidlattimore merged 1 commit intowild-linker:mainfrom
el-yawd:issue/1223
Oct 21, 2025
Merged

Add basic testing for update-in-place flag#1230
davidlattimore merged 1 commit intowild-linker:mainfrom
el-yawd:issue/1223

Conversation

@el-yawd
Copy link
Copy Markdown
Contributor

@el-yawd el-yawd commented Oct 19, 2025

The new test mode actually breaks, but I'm not sure if I'm testing it right. The test pass when random bytes overwrite section is commented out.

Copy link
Copy Markdown
Member

@davidlattimore davidlattimore left a comment

Choose a reason for hiding this comment

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

Thanks for this! Looks good. I've started investigating the spots were the file isn't being written. I'll hopefully finish that tomorrow at which point the test will hopefully pass.

I think it makes sense to do this as one PR, then once this one is done, do the switching to default, fallback etc as a second PR. That being the case, this PR won't close the linked issue.

"(no differences found - this shouldn't happen)".to_string()
};

bail!(
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.

Adding the filename of the binary would be useful, since the next thing someone would want to do is look at the binary to see what's at the specified offset in the file - e.g. what section it's in.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think in case of error it's better to have both files to debug and print the respective path

Comment thread wild/tests/integration_tests.rs Outdated
);
}

println!("Update-in-place test passed for {}", self.source_file);
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.

Probably no need to print output when it succeeds

@davidlattimore
Copy link
Copy Markdown
Member

I've fixed missing zero-fill bugs - #1237. If you rebase your change, then I think the tests should now pass.

@el-yawd
Copy link
Copy Markdown
Contributor Author

el-yawd commented Oct 20, 2025

I've fixed missing zero-fill bugs - #1237. If you rebase your change, then I think the tests should now pass.

Cool!

@el-yawd el-yawd marked this pull request as ready for review October 20, 2025 23:47
@davidlattimore davidlattimore merged commit c056435 into wild-linker:main Oct 21, 2025
20 checks passed
daniel-levin pushed a commit to daniel-levin/wild that referenced this pull request Oct 23, 2025
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