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

AhoCorasick::memory_usage doctests fail on i686 (and possibly most 32-bit architectures) #116

Closed
decathorpe opened this issue May 29, 2023 · 3 comments

Comments

@decathorpe
Copy link

I'm maintaining the Fedora Linux package for this crate, and upon the update from v0.7 to v1.0, I see new test failure on 32-bit x86 (i.e. i686-unknown-linux-gnu) related to AhoCorasick::memory_usage:

---- src/ahocorasick.rs - ahocorasick::AhoCorasick::memory_usage (line 1977) stdout ----
Test executable failed (exit status: 101).
stderr:
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `5632`,
 right: `5584`', src/ahocorasick.rs:10:1

The failure itself looks harmless (i.e. the memory usage is lower than the expected one, which isn't unexpected on 32-bit systems (as types like usize / isize etc. are half the size on i686 than they are on x86_64), but I wanted to report this nonetheless.

For now I'm going to ignore these tests on 32-bit systems, but maybe it might make sense to make the comparison <= expected instead of ==expected, or to gate the assertions in the AhoCorasick::memory_usage doctest with #[cfg(target_pointer_width = "64")]?

@BurntSushi
Copy link
Owner

So the thing that is perplexing to me is that CI specifically includes a 32-bit target, and yet these tests don't fail.

Playing around locally, it looks like perhaps the doc tests aren't running under cross. Sigh.

@BurntSushi
Copy link
Owner

BurntSushi added a commit that referenced this issue Jun 4, 2023
The specific values can change depending on pointer size, so we just
skip them for non-64-bit targets.

Fixes #116
@decathorpe
Copy link
Author

Oh, yeah, I did not expect that doctests don't get run in these circumstances ... thanks for investigating + fixing! 👍🏼

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

No branches or pull requests

2 participants