Skip to content

Commit

Permalink
Github Actions to build and test PRs (#61)
Browse files Browse the repository at this point in the history
This PR:

- Adds a `--check` flag for the code formatters in `xtask fmt`.
- Adds a `.github/workflows` action.
  • Loading branch information
jhugman authored Aug 14, 2024
1 parent d494612 commit 45aa9da
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 70 deletions.
46 changes: 46 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: Rust

on:
push:
branches: ["main"]
pull_request:
branches: ["main"]

env:
CARGO_TERM_COLOR: always

jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
# Build
- name: Build
run: cargo build --verbose

# Install tooling
- name: Install clang-format
run: sudo apt-get install -y clang-format
- name: Install yarn
run: cargo xtask bootstrap yarn

# Lint
- name: Check Typescript format
run: cargo xtask fmt --check typescript
- name: Check Rust format
run: cargo xtask fmt --check rust

# Unit tests
- name: Run tests
run: cargo test --verbose

# Integration tests
- name: Install tooling for building C++
run: sudo apt-get install -y cmake ninja-build
- name: Installing hermes and test-runner
run: cargo xtask bootstrap
- name: Run tests of generated bindings
run: ./scripts/run-tests.sh

- name: Done
run: echo "Success!"
Original file line number Diff line number Diff line change
Expand Up @@ -219,33 +219,6 @@ pub(crate) trait CodeType: std::fmt::Debug {
format!("FfiConverter{}", self.canonical_name())
}

// XXX - the below should be removed and replace with the ffi_converter_name reference in the template.
/// An expression for lowering a value into something we can pass over the FFI.
fn lower(&self) -> String {
format!("{}.lower", self.ffi_converter_name())
}

/// An expression for writing a value into a byte buffer.
fn write(&self) -> String {
format!("{}.write", self.ffi_converter_name())
}

/// An expression for lifting a value from something we received over the FFI.
fn lift(&self) -> String {
format!("{}.lift", self.ffi_converter_name())
}

/// An expression for reading a value from a byte buffer.
fn read(&self) -> String {
format!("{}.read", self.ffi_converter_name())
}

/// A list of imports that are needed if this type is in use.
/// Classes are imported exactly once.
fn imports(&self) -> Option<Vec<String>> {
None
}

/// Function to run at startup
fn initialization_fn(&self) -> Option<String> {
None
Expand Down
4 changes: 2 additions & 2 deletions crates/ubrn_bindgen/src/bindings/react_native/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl BindingGenerator for ReactNativeBindingGenerator {
}

fn format_ts(out_dir: &Utf8Path) -> Result<()> {
if let Some(mut prettier) = fmt::prettier(out_dir)? {
if let Some(mut prettier) = fmt::prettier(out_dir, false)? {
run_cmd_quietly(&mut prettier)?
} else {
eprintln!("No prettier found. Install with `yarn add --dev prettier`");
Expand All @@ -107,7 +107,7 @@ fn format_ts(out_dir: &Utf8Path) -> Result<()> {
}

fn format_cpp(out_dir: &Utf8Path) -> Result<()> {
if let Some(mut clang_format) = fmt::clang_format(out_dir)? {
if let Some(mut clang_format) = fmt::clang_format(out_dir, false)? {
run_cmd_quietly(&mut clang_format)?
} else {
eprintln!("Skipping formatting C++. Is clang-format installed?");
Expand Down
19 changes: 14 additions & 5 deletions crates/ubrn_common/src/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,36 @@ use which::which;

use crate::{file_paths, resolve};

pub fn clang_format<P: AsRef<Utf8Path>>(path: P) -> Result<Option<Command>> {
pub fn clang_format<P: AsRef<Utf8Path>>(path: P, check_only: bool) -> Result<Option<Command>> {
if which("clang-format").is_err() {
return Ok(None);
}

let path = path.as_ref();
let mut cmd = Command::new("clang-format");
cmd.arg("-i")
.arg("--style=file")
if check_only {
cmd.arg("--dry-run").arg("--Werror");
} else {
cmd.arg("-i");
}
cmd.arg("--style=file")
.arg("--fallback-style=LLVM")
.args(file_paths(&format!("{path}/**/*.[ch]"))?)
.args(file_paths(&format!("{path}/**/*.[ch]pp"))?)
.current_dir(path);
Ok(Some(cmd))
}

pub fn prettier<P: AsRef<Utf8Path>>(out_dir: P) -> Result<Option<Command>> {
pub fn prettier<P: AsRef<Utf8Path>>(out_dir: P, check_only: bool) -> Result<Option<Command>> {
let prettier = resolve(&out_dir, "node_modules/.bin/prettier")?;
Ok(if let Some(prettier) = prettier {
let mut cmd = Command::new(prettier);
cmd.arg(".").arg("--write").current_dir(out_dir.as_ref());
if check_only {
cmd.arg("--check");
} else {
cmd.arg("--write");
}
cmd.arg(".").current_dir(out_dir.as_ref());
Some(cmd)
} else {
None
Expand Down
4 changes: 0 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@
"ubrn": "./bin/cli",
"uniffi-bindgen-react-native": "./bin/cli"
},
"scripts": {
"build": "tsc",
"prepare": "yarn build"
},
"devDependencies": {
"metro": "^0.80.8",
"metro-core": "^0.80.8",
Expand Down
77 changes: 45 additions & 32 deletions xtask/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::process::Command;

use anyhow::Result;
use clap::{Args, Subcommand};
use ubrn_common::run_cmd;
use ubrn_common::{run_cmd, run_cmd_quietly};

use crate::{
bootstrap::{Bootstrap, YarnCmd},
Expand All @@ -16,26 +16,29 @@ use crate::{

#[derive(Debug, Args)]
pub(crate) struct FmtArgs {
/// If set, only check, otherwise format files in place.
#[clap(long)]
check: bool,
#[clap(subcommand)]
cmd: Option<LanguageCmd>,
}

pub(crate) trait CodeFormatter {
fn format_code(&self) -> Result<()>;
fn format_code(&self, check_only: bool) -> Result<()>;
}

impl FmtArgs {
pub(crate) fn run(&self) -> Result<()> {
self.format_code()
self.format_code(self.check)
}
}

impl CodeFormatter for FmtArgs {
fn format_code(&self) -> Result<()> {
fn format_code(&self, check_only: bool) -> Result<()> {
if let Some(c) = &self.cmd {
c.format_code()
c.format_code(check_only)
} else {
LanguageCmd::format_all()
LanguageCmd::format_all(check_only)
}
}
}
Expand Down Expand Up @@ -67,24 +70,24 @@ enum LanguageCmd {
}

impl CodeFormatter for LanguageCmd {
fn format_code(&self) -> Result<()> {
fn format_code(&self, check_only: bool) -> Result<()> {
match self {
Self::Rust(c) => c.format_code()?,
Self::Typescript(c) => c.format_code()?,
Self::Cpp(c) => c.format_code()?,
Self::Licence(c) => c.format_code()?,
Self::Rust(c) => c.format_code(check_only)?,
Self::Typescript(c) => c.format_code(check_only)?,
Self::Cpp(c) => c.format_code(check_only)?,
Self::Licence(c) => c.format_code(check_only)?,
}
Ok(())
}
}

impl LanguageCmd {
fn format_all() -> Result<()> {
fn format_all(check_only: bool) -> Result<()> {
// We add to the source code, before formatting.
LicenceArgs.format_code()?;
RustArgs::default().format_code()?;
TypescriptArgs.format_code()?;
CppArgs.format_code()?;
LicenceArgs.format_code(check_only)?;
RustArgs::default().format_code(check_only)?;
TypescriptArgs.format_code(check_only)?;
CppArgs.format_code(check_only)?;
Ok(())
}
}
Expand All @@ -101,22 +104,25 @@ struct RustArgs {
}

impl CodeFormatter for RustArgs {
fn format_code(&self) -> Result<()> {
fn format_code(&self, check_only: bool) -> Result<()> {
let root = repository_root()?;
let run_fmt = !self.only_clippy;
let run_clippy = self.only_clippy || !self.no_clippy;
if run_fmt {
run_cmd(
Command::new("cargo")
.arg("fmt")
.arg("--all")
.current_dir(&root),
)?;
let mut cmd = Command::new("cargo");
cmd.arg("--quiet")
.arg("fmt")
.arg("--all")
.current_dir(&root);
if check_only {
cmd.arg("--check");
}
run_cmd_quietly(&mut cmd)?;
}

if run_clippy {
run_cmd(
Command::new("cargo")
.arg("--quiet")
.arg("clippy")
.arg("--all")
.current_dir(root),
Expand All @@ -131,11 +137,11 @@ impl CodeFormatter for RustArgs {
struct TypescriptArgs;

impl CodeFormatter for TypescriptArgs {
fn format_code(&self) -> Result<()> {
fn format_code(&self, check_only: bool) -> Result<()> {
YarnCmd.ensure_ready()?;
let root = repository_root()?;
if let Some(mut prettier) = ubrn_common::fmt::prettier(root)? {
run_cmd(&mut prettier)?
if let Some(mut prettier) = ubrn_common::fmt::prettier(root, check_only)? {
run_cmd_quietly(&mut prettier)?
} else {
unreachable!("Is prettier in package.json dependencies?")
}
Expand All @@ -147,10 +153,12 @@ impl CodeFormatter for TypescriptArgs {
struct CppArgs;

impl CodeFormatter for CppArgs {
fn format_code(&self) -> Result<()> {
fn format_code(&self, check_only: bool) -> Result<()> {
let root = repository_root()?;
if let Some(mut clang_format) = ubrn_common::fmt::clang_format(root.join("cpp"))? {
run_cmd(&mut clang_format)?;
if let Some(mut clang_format) =
ubrn_common::fmt::clang_format(root.join("cpp"), check_only)?
{
run_cmd_quietly(&mut clang_format)?;
} else {
eprintln!("clang-format doesn't seem to be installed")
}
Expand All @@ -162,10 +170,15 @@ impl CodeFormatter for CppArgs {
struct LicenceArgs;

impl CodeFormatter for LicenceArgs {
fn format_code(&self) -> Result<()> {
fn format_code(&self, check_only: bool) -> Result<()> {
if check_only {
// source-licenser doesn't provide a check only.
// Rather than changing the files on disk, just return.
return Ok(());
}
YarnCmd.ensure_ready()?;
let root = repository_root()?;
run_cmd(
run_cmd_quietly(
Command::new("./node_modules/.bin/source-licenser")
.arg(".")
.arg("--config-file")
Expand Down

0 comments on commit 45aa9da

Please sign in to comment.