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

Add a test that an odd capacity BufEncoder does not compile #157

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ Run from rust.yml unless stated otherwise. Unfortunately we are now exceeding th
10. `Arch32bit`
11. `Cross`
12. `Format`
13. `Compile tests`
11 changes: 11 additions & 0 deletions .github/workflows/rust.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,14 @@ jobs:
run: rustup component add rustfmt
- name: "Check formatting"
run: cargo +nightly fmt --all -- --check

Compile:
name: Compile tests
runs-on: ubuntu-latest
steps:
- name: "Checkout repo"
uses: actions/checkout@v4
- name: "Select toolchain"
uses: dtolnay/rust-toolchain@stable
- name: "Run compile tests"
run: ./contrib/compile-tests.sh
43 changes: 43 additions & 0 deletions contrib/compile-tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#!/usr/bin/env bash
#
# Checks intended compile failures.

set -euo pipefail

REPO_DIR=$(git rev-parse --show-toplevel)

compile_check() {
local dir_path=$1
local expected_status=$2
local result=0
local output
# All crates in the specified subdirectory must compile or fail based on expected_status
for dir in "$dir_path"/*; do
if [ -d "$dir" ]; then
pushd "$dir" > /dev/null
echo "Compiling $dir"
output=$(cargo build 2>&1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be needed but I guess it's OK. I don't expect the output to be too large.

if [ "$?" -ne "$expected_status" ]; then
if [ "$expected_status" -eq 0 ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why have another argument if it can be inferred from the first one and you're not comparing them anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started to change it, but it's more flexible the way it is. I'm not sure we need to but it's possible to check directories other than pass/ and fail/ by passing a different argument to the function.

But if you prefer I can hard code the directory names and exit codes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Too many possibilities that we're not using are just complicating the code. Honestly, I think even having the pass option is a stretch (normal tests do that already!) but I understand it's for the symmetry since there are related test (one for 3, other 4) so I'm accepting it. I was really expecting this to be one line in the workflow file.

echo "$output"
echo "error: compile-tests/$dir/ failed to compile"
else
echo "error: compile-tests/$dir/ compiled when it should not have"
fi
result=1
fi
popd > /dev/null
fi
done
return "$result"
}

# Check that all files in compiletests pass or fail as expected
cd "$REPO_DIR"/tests/compile-tests
if compile_check "pass" 0 && compile_check "fail" 101; then
echo "Compile tests passed"
exit 0
else
echo "Compile tests failed"
exit 1
fi