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

Ensures make tests run under /bin/dash (if available), like CI, and fixes a Makefile #81734

Merged
merged 4 commits into from
Feb 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/test/run-make-fulldeps/coverage-reports/Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# ignore-test Broken; accidentally silently ignored on Linux CI; FIXME(#81688)
# needs-profiler-support
# ignore-windows-gnu
# min-llvm-version: 11.0
Expand Down Expand Up @@ -128,7 +127,7 @@ endif
$$( \
for file in $(TMPDIR)/rustdoc-$@/*/rust_out; \
do \
[[ -x $$file ]] && printf "%s %s " -object $$file; \
[ -x "$$file" ] && printf "%s %s " -object $$file; \
done \
) \
2> "$(TMPDIR)"/[email protected] \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
2| |// structure of this test.
3| |
4| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
^0 ^0 ^0 ^0 ^1 ^0 ^0^0
^0 ^0 ^0 ^0 ^1 ^1 ^0^0
5| |pub struct Version {
6| | major: usize,
7| 1| minor: usize, // Count: 1 - `PartialOrd` compared `minor` values in 3.2.1 vs. 3.3.0
8| 0| patch: usize, // Count: 0 - `PartialOrd` was determined by `minor` (2 < 3)
7| | minor: usize,
8| | patch: usize,
9| |}
10| |
11| |impl Version {
Expand Down Expand Up @@ -45,19 +45,4 @@
44| |`function_source_hash` without a code region, if necessary.
45| |
46| |*/
47| |
48| |// FIXME(#79626): The derived traits get coverage, which is great, but some of the traits appear
49| |// to get two coverage execution counts at different positions:
50| |//
51| |// ```text
52| |// 4| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
53| |// ^0 ^0 ^0 ^0 ^1 ^0 ^0^0
54| |// ```text
55| |//
56| |// `PartialEq`, `PartialOrd`, and `Ord` (and possibly `Eq`, if the trait name was longer than 2
57| |// characters) have counts at their first and last characters.
58| |//
59| |// Why is this? Why does `PartialOrd` have two values (1 and 0)? This must mean we are checking
60| |// distinct coverages, so maybe we don't want to eliminate one of them. Should we merge them?
61| |// If merged, do we lose some information?

2 changes: 0 additions & 2 deletions src/test/run-make-fulldeps/coverage/coverage_tools.mk
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,3 @@
# Enabling `-C link-dead-code` is not necessary when compiling with `-Z instrument-coverage`,
# due to improvements in the coverage map generation, to add unreachable functions known to Rust.
# Therefore, `-C link-dead-code` is no longer automatically enabled.

UNAME = $(shell uname)
19 changes: 2 additions & 17 deletions src/test/run-make-fulldeps/coverage/partial_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub struct Version {
major: usize,
minor: usize, // Count: 1 - `PartialOrd` compared `minor` values in 3.2.1 vs. 3.3.0
patch: usize, // Count: 0 - `PartialOrd` was determined by `minor` (2 < 3)
minor: usize,
patch: usize,
}

impl Version {
Expand Down Expand Up @@ -44,18 +44,3 @@ one expression, which is allowed, but the `function_source_hash` was only passed
`function_source_hash` without a code region, if necessary.

*/

// FIXME(#79626): The derived traits get coverage, which is great, but some of the traits appear
// to get two coverage execution counts at different positions:
//
// ```text
// 4| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
// ^0 ^0 ^0 ^0 ^1 ^0 ^0^0
// ```text
//
// `PartialEq`, `PartialOrd`, and `Ord` (and possibly `Eq`, if the trait name was longer than 2
// characters) have counts at their first and last characters.
//
// Why is this? Why does `PartialOrd` have two values (1 and 0)? This must mean we are checking
// distinct coverages, so maybe we don't want to eliminate one of them. Should we merge them?
// If merged, do we lose some information?
15 changes: 15 additions & 0 deletions src/test/run-make-fulldeps/tools.mk
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,21 @@ CGREP := "$(S)/src/etc/cat-and-grep.sh"
# diff with common flags for multi-platform diffs against text output
DIFF := diff -u --strip-trailing-cr

# Some of the Rust CI platforms use `/bin/dash` to run `shell` script in
# Makefiles. Other platforms, including many developer platforms, default to
# `/bin/bash`. (In many cases, `make` is actually using `/bin/sh`, but `sh`
# is configured to execute one or the other shell binary). `dash` features
# support only a small subset of `bash` features, so `dash` can be thought of as
# the lowest common denominator, and tests should be validated against `dash`
# whenever possible. Most developer platforms include `/bin/dash`, but to ensure
# tests still work when `/bin/dash`, if not available, this `SHELL` override is
# conditional:
ifndef IS_WINDOWS # dash interprets backslashes in executable paths incorrectly
ifneq (,$(wildcard /bin/dash))
SHELL := /bin/dash
endif
endif

# This is the name of the binary we will generate and run; use this
# e.g. for `$(CC) -o $(RUN_BINFILE)`.
RUN_BINFILE = $(TMPDIR)/$(1)
Expand Down