Enable ignoring tests based on architecture. #19809
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see CONTRIBUTING.md for more information. |
| @@ -34,6 +43,15 @@ pub fn get_os(triple: &str) -> &'static str { | |||
| panic!("Cannot determine OS from triple"); | |||
| } | |||
|
|
|||
| pub fn get_arch(triple: &str) -> &'static str { | |||
| for &triple_arch in ARCH_TABLE.iter() { | |||
| if triple.contains(triple_arch) { | |||
There was a problem hiding this comment.
Because x86_64 contains x86, doesn't this mean that // ignore-x86 will ignore 32 and 64 bit platforms?
…h can correctly determine the architecture. (So, x86_64 will not be identified as x86, and mipsel will not be identified as mips.)
|
You're right, I've missed that case. (And the same applies to mipsel/mips). So, I've reordered ARCH_TABLE so that substrings come later. This way, get_arch() will check for and find x86_64 first. (Also, I forgot to update the copyright line in the touched files, which is suggested by the guidelines. Fixed that now.) |
|
With just a check of |
|
In case of Another alternative would be Unfortunately, you may still be right, because of |
|
Triples are known to have a |
…le by splitting at '-' (instead of relying on tables and .contains). This makes previous changes to util.rs unnecessary.
| @@ -209,7 +213,8 @@ pub fn is_test_ignored(config: &Config, testfile: &Path) -> bool { | |||
|
|
|||
| let val = iter_header(testfile, |ln| { | |||
| !parse_name_directive(ln, "ignore-test") && | |||
| !parse_name_directive(ln, ignore_target(config).as_slice()) && | |||
| !parse_name_directive(ln, ignore_os(config).as_slice()) && | |||
| !parse_name_directive(ln, ignore_arch(config).as_slice()) && | |||
There was a problem hiding this comment.
Unfortunately due to the use of .contains in parse_name_directive I think this suffers the same problem as before :(
|
@akiss77 status? |
|
Closing due to inactivity. Feel free to update and reopen, though! |
internal: Catch panics in inference in analysis-stats
Now, tests can only be ignored on OS basis. However, some tests - e.g., compile-fail/asm-misplaced-option.rs - should be skipped because of the target architecture.
(Note: in the case of compile-fail/asm-misplaced-option.rs, #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] pub fn main() prevents the Intel assembly to be processed on another architecture but does not prevent compiletest to find and expect the warnings within main.)
So, this PR proposes to enhance compiletest to recognize // ignore-ARCH lines as well.