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

ci: don't special case benchmark testing #1568

Merged
merged 1 commit into from Feb 20, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 0 additions & 8 deletions .github/workflows/linux.yml
Expand Up @@ -128,14 +128,6 @@ jobs:
# name: coverage-diff.html
# path: coverage-diff.html

benchmark:
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@v4
- run: ./scripts/install_zig.sh
- run: ./zig/zig build -Drelease -Dconfig=production run -- benchmark --transfer-count=4000

aof:
runs-on: ubuntu-latest
steps:
Expand Down
11 changes: 0 additions & 11 deletions .github/workflows/macos.yml
Expand Up @@ -18,17 +18,6 @@ jobs:
- run: ./zig/zig build test
- run: ./scripts/install.sh

benchmark:
timeout-minutes: 10
strategy:
matrix:
target: [macos-13]
runs-on: ${{ matrix.target }}
steps:
- uses: actions/checkout@v4
- run: ./scripts/install_zig.sh
- run: ./zig/zig build -Drelease -Dconfig=production run -- benchmark --transfer-count=4000

c_sample:
strategy:
matrix:
Expand Down
7 changes: 0 additions & 7 deletions .github/workflows/windows.yml
Expand Up @@ -4,13 +4,6 @@ on:
workflow_call:

jobs:
benchmark:
runs-on: windows-latest
steps:
- uses: actions/checkout@v4
- run: .\scripts\install_zig.bat
- run: .\zig\zig build -Drelease -Dconfig=production run -- benchmark --transfer-count=4000

c_sample:
runs-on: windows-latest
steps:
Expand Down
12 changes: 12 additions & 0 deletions src/integration_tests.zig
Expand Up @@ -144,3 +144,15 @@ test "repl integration" {
\\
));
}

test "benchmark smoke" {
const shell = try Shell.create(std.testing.allocator);
defer shell.destroy();

const tigerbeetle = try tigerbeetle_exe(shell);
const status_ok = try shell.exec_status_ok(
"{tigerbeetle} benchmark --transfer-count=4000",
.{ .tigerbeetle = tigerbeetle },
);
try std.testing.expect(status_ok);
}
4 changes: 2 additions & 2 deletions src/shell.zig
Expand Up @@ -388,7 +388,7 @@ pub fn exec_options(
}
}

/// Returns `true` if the command executed successfully with a zero exit code and an empty stderr.
/// Returns `true` if the command executed successfully with a zero exit code.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the second time the stderr check gets in the way (as benchmark logs to stderr). Before that, I get a false error somewhere where I was running tool --version, and was getting print out to stderr about the tool being installed transparently by some sort of version manager. So I figured it's better to keep it simple and just not check for stderr.

///
/// One intended use-case is sanity-checking that an executable is present, by running
/// `my-tool --version`.
Expand All @@ -411,7 +411,7 @@ pub fn exec_status_ok(shell: *Shell, comptime cmd: []const u8, cmd_args: anytype
defer shell.gpa.free(res.stdout);

return switch (res.term) {
.Exited => |code| code == 0 and res.stderr.len == 0,
.Exited => |code| code == 0,
else => false,
};
}
Expand Down
13 changes: 9 additions & 4 deletions src/tigerbeetle/benchmark_driver.zig
Expand Up @@ -22,11 +22,16 @@ const benchmark_load = @import("./benchmark_load.zig");

const log = std.log;

// Note: we intentionally don't use a temporary directory for this data file, and instead just put
// it into CWD, as performance of TigerBeetle very much depends on a specific file system.
const data_file = "0_0.tigerbeetle.benchmark";

pub fn main(allocator: std.mem.Allocator, args: *const cli.Command.Benchmark) !void {
// Note: we intentionally don't use a temporary directory for this data file, and instead just
// put it into CWD, as performance of TigerBeetle very much depends on a specific file system.
const data_file = data_file: {
var random_bytes: [4]u8 = undefined;
std.crypto.random.bytes(&random_bytes);
const random_suffix: [8]u8 = std.fmt.bytesToHex(random_bytes, .lower);
break :data_file "0_0-" ++ random_suffix ++ ".tigerbeetle.benchmark";
};

var data_file_created = false;
defer {
if (data_file_created) {
Expand Down