github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 18:18:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

test(gui-client): fix zip file export in smoke tests (#5465)

Browse Source 
Closes #5464

These were silently broken, it was exporting an empty zip and passing
the test anyway. So this PR will cause the test to fail if the zip
wasn't fully exported, and then it will fix the export.
This commit is contained in:
Reactor Scram
2024-06-21 14:05:26 +00:00
committed by GitHub
parent eb7b3f62ab
commit b8f92ed812
4 changed files with 28 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
rust/gui-client/src-tauri/src/client/gui.rs
View File

@@ -170,8 +170,7 @@ pub(crate) fn run(
let ctlr_tx = ctlr_tx.clone();
tokio::spawn(async move {
if let Err(error) = smoke_test(ctlr_tx).await {
tracing::error!(?error, "Error during smoke test");
tracing::error!("Crashing on purpose so a dev can see our stacktraces");
tracing::error!(?error, "Error during smoke test, crashing on purpose so a dev can see our stacktraces");
unsafe { sadness_generator::raise_segfault() }
}
});
@@ -324,8 +323,8 @@ async fn smoke_test(ctlr_tx: CtlrTx) -> Result<()> {
.await
.context("Failed to get zip file metadata")?
.len();
if zip_len == 0 {
bail!("Exported log zip has 0 bytes");
if zip_len <= 22 {
bail!("Exported log zip just has the file header");
}
tokio::fs::remove_file(&path)
.await

9
rust/gui-client/src-tauri/src/client/logging.rs
View File

@@ -82,7 +82,7 @@ pub(crate) async fn clear_logs() -> StdResult<(), String> {
#[tauri::command]
pub(crate) async fn export_logs(managed: tauri::State<'_, Managed>) -> StdResult<(), String> {
export_logs_inner(managed.ctlr_tx.clone()).map_err(|e| e.to_string())
show_export_dialog(managed.ctlr_tx.clone()).map_err(|e| e.to_string())
}
#[derive(Clone, Default, Serialize)]
@@ -112,7 +112,7 @@ pub(crate) async fn clear_logs_inner() -> Result<()> {
}
/// Pops up the "Save File" dialog
pub(crate) fn export_logs_inner(ctlr_tx: CtlrTx) -> Result<()> {
fn show_export_dialog(ctlr_tx: CtlrTx) -> Result<()> {
let now = chrono::Local::now();
let datetime_string = now.format("%Y_%m_%d-%H-%M");
let stem = PathBuf::from(format!("firezone_logs_{datetime_string}"));
@@ -144,15 +144,18 @@ pub(crate) fn export_logs_inner(ctlr_tx: CtlrTx) -> Result<()> {
/// * `stem` - A directory containing all the log files inside the zip archive, to avoid creating a ["tar bomb"](https://www.linfo.org/tarbomb.html). This comes from the automatically-generated name of the archive, even if the user changes it to e.g. `logs.zip`
pub(crate) async fn export_logs_to(path: PathBuf, stem: PathBuf) -> Result<()> {
tracing::info!("Exporting logs to {path:?}");
// Use a temp path so that if the export fails we don't end up with half a zip file
let temp_path = path.with_extension(".zip-partial");
// TODO: Consider https://github.com/Majored/rs-async-zip/issues instead of `spawn_blocking`
spawn_blocking(move || {
let f = fs::File::create(path).context("Failed to create zip file")?;
let f = fs::File::create(&temp_path).context("Failed to create zip file")?;
let mut zip = zip::ZipWriter::new(f);
for log_path in log_paths().context("Can't compute log paths")? {
add_dir_to_zip(&mut zip, &log_path.src, &stem.join(log_path.dst))?;
}
zip.finish().context("Failed to finish zip file")?;
fs::rename(&temp_path, &path)?;
Ok::<_, anyhow::Error>(())
})
.await

15
scripts/tests/smoke-test-gui-linux.sh
View File

@@ -1,15 +1,20 @@
#!/usr/bin/env bash
# Put `set -euox` in the top-level scripts directly. If it's only sourced,
# and the source path is wrong, it will not throw an error and it'll be hard
# to debug.
# <https://github.com/firezone/firezone/actions/runs/9602401296/job/26483176628#step:11:12>
set -euox pipefail
BUNDLE_ID="dev.firezone.client"
FZ_GROUP="firezone-client"
#DEVICE_ID_PATH="/var/lib/$BUNDLE_ID/config/firezone-id.json"
LOGS_PATH="$HOME/.cache/$BUNDLE_ID/data/logs"
DUMP_PATH="$LOGS_PATH/last_crash.dmp"
SETTINGS_PATH="$HOME/.config/$BUNDLE_ID/config/advanced_settings.json"
IPC_LOGS_PATH="/var/log/$BUNDLE_ID"
RAN_BEFORE_PATH="$HOME/.local/share/$BUNDLE_ID/data/ran_before.txt"
SETTINGS_PATH="$HOME/.config/$BUNDLE_ID/config/advanced_settings.json"
SYMS_PATH="../target/debug/firezone-gui-client.syms"
PACKAGE=firezone-gui-client
@@ -26,6 +31,9 @@ ls -lash ../target/debug
sudo groupadd --force "$FZ_GROUP"
sudo adduser "$USER" "$FZ_GROUP"
# Make the IPC log dir so that the zip export doesn't bail out
sudo mkdir -p "$IPC_LOGS_PATH"
function run_fz_gui() {
pwd
# Does what it says
@@ -55,7 +63,8 @@ function smoke_test() {
# Make sure the files were written in the right paths
# TODO: Inject some bogus sign-in sequence to test the actor_name file
# https://stackoverflow.com/questions/41321092
bash -c "stat \"${LOGS_PATH}/\"connlib*log"
# TODO: Smoke test the IPC service
# bash -c "stat \"${LOGS_PATH}/\"connlib*log"
stat "$SETTINGS_PATH"
# stat "$DEVICE_ID_PATH"
# `ran_before` is now only written after a successful sign-in

25
scripts/tests/smoke-test-gui-windows.sh
View File

@@ -3,7 +3,7 @@
# Read it before running on a dev system.
# This script must run from an elevated shell so that Firezone won't try to elevate.
source "./scripts/tests/lib.sh"
set -euox pipefail
# This prevents a `shellcheck` lint warning about using an unset CamelCase var
if [[ -z "$ProgramData" ]]; then
@@ -12,15 +12,18 @@ if [[ -z "$ProgramData" ]]; then
fi
BUNDLE_ID="dev.firezone.client"
DEVICE_ID_PATH="$ProgramData/$BUNDLE_ID/config/firezone-id.json"
DUMP_PATH="$LOCALAPPDATA/$BUNDLE_ID/data/logs/last_crash.dmp"
IPC_LOGS_PATH="$ProgramData/$BUNDLE_ID/data/logs"
PACKAGE=firezone-gui-client
# Make the IPC log dir so that the zip export doesn't bail out
mkdir -p "$IPC_LOGS_PATH"
function smoke_test() {
# This array used to have more items
# TODO: Smoke-test the IPC service
files=(
"$LOCALAPPDATA/$BUNDLE_ID/config/advanced_settings.json"
"$LOCALAPPDATA/$BUNDLE_ID/data/wintun.dll"
"$DEVICE_ID_PATH"
)
# Make sure the files we want to check don't exist on the system yet
@@ -34,25 +37,11 @@ function smoke_test() {
# Run the smoke test normally
cargo run --bin "$PACKAGE" -- smoke-test
# Note the device ID
DEVICE_ID_1=$(cat "$DEVICE_ID_PATH")
# Run the test again and make sure the device ID is not changed
cargo run --bin "$PACKAGE" -- smoke-test
DEVICE_ID_2=$(cat "$DEVICE_ID_PATH")
if [ "$DEVICE_ID_1" != "$DEVICE_ID_2" ]
then
echo "The device ID should not change if the file is intact between runs"
exit 1
fi
# Make sure the files were written in the right paths
for file in "${files[@]}"
do
stat "$file"
done
stat "$LOCALAPPDATA/$BUNDLE_ID/data/logs/"connlib*log
# Clean up so the test can be cycled
for file in "${files[@]}"