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

chore(libsinsp/test): use GTEST_SKIP to track disabled tests #2158

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ jobs:
build-libs-linux:
name: build-libs-linux-${{ matrix.arch }} 😁 (${{ matrix.name }})
runs-on: ${{ (matrix.arch == 'arm64' && 'github-arm64-2c-8gb') || 'ubuntu-22.04' }}
container:
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

At this point, do we even need the container? I mean, we already run on ubuntu-22.04 on x86 and arm64.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd remove the container directive.
Also, remember to add sudo before install deps steps

image: ubuntu:22.04
strategy:
fail-fast: false
matrix:
Expand All @@ -33,12 +35,10 @@ jobs:
cmake_opts: -DUSE_ASAN=On -DUSE_UBSAN=On -DUSE_BUNDLED_DEPS=False
- name: zig
cmake_opts: -DUSE_BUNDLED_DEPS=True
container:
image: debian:buster
steps:
- name: Install deps ⛓️
run: |
apt update && apt install -y --no-install-recommends curl ca-certificates build-essential git clang llvm pkg-config autoconf automake libtool libelf-dev wget libc-ares-dev libcurl4-openssl-dev libssl-dev libtbb-dev libjq-dev libjsoncpp-dev libgrpc++-dev protobuf-compiler-grpc libgtest-dev libprotobuf-dev linux-headers-${{ matrix.arch }}
apt update && apt install -y --no-install-recommends curl ca-certificates build-essential git clang llvm pkg-config autoconf automake libtool libelf-dev wget libc-ares-dev libcurl4-openssl-dev libssl-dev libtbb-dev libjq-dev libjsoncpp-dev libgrpc++-dev protobuf-compiler-grpc libgtest-dev libprotobuf-dev linux-headers-$(uname -r)

- name: Install a recent version of CMake ⛓️
run: |
Expand Down
17 changes: 8 additions & 9 deletions userspace/libsinsp/test/async_key_value_source.ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,16 +335,16 @@ TEST(async_key_value_source_test, look_key_delayed_async_callback) {

/**
* Ensure that "old" results are pruned
* This test usually fails like this when runned with sanitizers on arm64:
*
* pure virtual method called
* terminate called without an active exception
* Aborted (core dumped)
*
* Disabled until we can figure out how to fix it.
*/
#if !defined(__aarch64__)
TEST(async_key_value_source_test, prune_old_metadata) {
#if defined(__aarch64__)
GTEST_SKIP() << R"(This test usually fails like this when runned with sanitizers on arm64:
pure virtual method called
terminate called without an active exception
Aborted (core dumped)

Disabled until we can figure out how to fix it.)";
#endif
const uint64_t DELAY_MS = 0;
const uint64_t TTL_MS = 20;

Expand Down Expand Up @@ -380,7 +380,6 @@ TEST(async_key_value_source_test, prune_old_metadata) {
// fetch the first key should also return false.
ASSERT_FALSE(source.lookup(key1, response));
}
#endif

struct result {
uint64_t val = 0;
Expand Down
Loading