diff --git a/CHANGELOG.md b/CHANGELOG.md index 85bbdc3..8e49761 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Regex based parser has been removed in favor of a `winnow` based parser combinator, the format is now more strict. See [SPEC.md](spec.md) for more details. ([#255](https://github.com/heroku/buildpacks-procfile/pull/255)) + - Keys starting with spaces now emit a warning + - Underscore key characters (`_`) are now converted to hyphens (`-`) and emit a warning + - Uppercase key characters are now converted to lowercase and emit a warning + - Invalid keys now error, previously they were ignored + ## [3.2.0] - 2024-12-20 ### Changed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6a6236b..226a39c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,6 +5,10 @@ the recommendations and requirements for how to best contribute to Heroku Cloud Native Buildpacks. We strive to obey these as best as possible. As always, thanks for contributing. +## CNB Procfile format + +The format of a Procfile is defined in the [SPEC.md](SPEC.md) file. If parsing behavior differs between that description it is a bug and either the specification or parser must be updated. + ## Governance Model: Salesforce Sponsored The intent and goal of open sourcing this project is to increase the contributor diff --git a/Cargo.lock b/Cargo.lock index 5864b45..6164c73 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "aho-corasick" @@ -26,6 +26,22 @@ dependencies = [ "libc", ] +[[package]] +name = "annotate-snippets" +version = "0.11.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "710e8eae58854cdc1790fcb56cca04d712a17be849eeb81da2a724bf4bae2bc4" +dependencies = [ + "anstyle", + "unicode-width", +] + +[[package]] +name = "anstyle" +version = "1.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55cc3b69f167a1ef2e161439aa98aed94e6028e5f9a59be9a6ffb47aef1651f9" + [[package]] name = "async-trait" version = "0.1.83" @@ -685,6 +701,7 @@ dependencies = [ name = "procfile-buildpack" version = "0.0.0" dependencies = [ + "annotate-snippets", "bullet_stream", "fs-err", "indoc", @@ -692,7 +709,7 @@ dependencies = [ "libcnb-test", "libherokubuildpack", "linked-hash-map", - "regex", + "winnow", ] [[package]] @@ -969,6 +986,12 @@ version = "1.0.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "adb9e6ca4f869e1180728b7950e35922a7fc6397f7b641499e8f3ef06e50dc83" +[[package]] +name = "unicode-width" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fc81956842c57dac11422a97c3b8195a1ff727f06e85c84ed2e8aa277c9a0fd" + [[package]] name = "uriparse" version = "0.6.4" @@ -1160,9 +1183,9 @@ checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" [[package]] name = "winnow" -version = "0.6.20" +version = "0.6.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "36c1fec1a2bb5866f07c25f68c26e565c4c200aebb96d7e55710c19d3e8ac49b" +checksum = "39281189af81c07ec09db316b302a3e67bf9bd7cbf6c820b50e35fee9c2fa980" dependencies = [ "memchr", ] diff --git a/Cargo.toml b/Cargo.toml index 396173f..a4a5b2b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,13 +14,14 @@ pedantic = { level = "warn", priority = -1 } unwrap_used = "warn" [dependencies] +annotate-snippets = "0.11.5" bullet_stream = "0.3" fs-err = "3" indoc = "2" libcnb = { version = "0.26", features = ["trace"] } libherokubuildpack = { version = "0.26", default-features = false, features = ["error", "log"] } linked-hash-map = "0.5" -regex = "1" +winnow = "0.6.22" [dev-dependencies] libcnb-test = "0.26" diff --git a/SPEC.md b/SPEC.md new file mode 100644 index 0000000..a864d3e --- /dev/null +++ b/SPEC.md @@ -0,0 +1,41 @@ +# CNB Procfile format specification + +This document outlines the format of a `Procfile`, which defines names to process types. For example: + +``` +web: rails s +worker: bundle exec sidekiq +``` + +## Differences from Heroku "classic" Procfile + +The classic `Procfile` has no formal specification. It is loosely defined based on a regex `"^[[:space:]]*([a-zA-Z0-9_-]+):?\\s+(.*)[[:space:]]*`. This specification is informed by the CNB specification for process names and [kubernetes](https://github.com/heroku/buildpacks-procfile/issues/251). + +## Specification + +The keywords "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt). + +- Spaces + - The term spaces refers to non-newline whitespace characters such as tab `'\t'` or space `'\s'` characters. +- Each line MUST contain a comment, empty line, or key/value pair. +- Comments + - A line MAY contain a comment + - Comments MUST contain a `#` as the first visible character on each line. + - A comment MAY be proceeded by one or more spaces. +- Empty line + - A Procfile MAY contain empty lines + - An empty line MUST be zero or more spaces followed by a line ending or end of file (EOF). +- Key/Value pairs + - A line MAY contain a key/value pair where the key represents the name of a process and the value represents a command + - A key MUST be separated from its value by a colon (`:`) followed by zero or more spaces. + - Duplicate keys MUST be allowed and the last entry MUST take precedence. A warning SHOULD be issued. +- Key + - A key's first and last character MUST be a lowercase alphanumeric (a-z0-9) character (but not `-`). + - All other key (middle) characters MUST be lowercase alphanumeric (a-z0-9) characters or hyphen `-`. + - Key length MUST be within the range `1..=63` + - An implementation MAY accept `_` as a middle character provided it converts it to `-` and issues a warning. + - An implementation MAY accept an uppercase character provided it is converted to lowercase characters and issues a warning. + - A key MAY be preceded with zero or more spaces provided they are not included in the return key and a warning is issued. +- Value + - A value MUST contain 1 or more non-whitespace characters. + - A value MUST be terminated by a newline or EOF. diff --git a/src/error.rs b/src/error.rs index 28e1dcf..66ed593 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,12 +1,12 @@ use crate::launch::ProcfileConversionError; -use crate::procfile::ProcfileParsingError; +use crate::procfile::ProcfileError; use bullet_stream::Print; use indoc::formatdoc; #[derive(Debug)] pub(crate) enum ProcfileBuildpackError { CannotReadProcfileContents(std::io::Error), - ProcfileParsingError(ProcfileParsingError), + ProcfileParsingError(ProcfileError), ProcfileConversionError(ProcfileConversionError), } @@ -23,9 +23,16 @@ pub(crate) fn error_handler(buildpack_error: ProcfileBuildpackError) { Underlying cause was: {io_error} "}); } - // There are currently no ways in which parsing can fail, however we will add some in the future: - // https://github.com/heroku/buildpacks-procfile/issues/73 - ProcfileBuildpackError::ProcfileParsingError(parsing_error) => match parsing_error {}, + ProcfileBuildpackError::ProcfileParsingError(parsing_error) => + build_output.error(formatdoc! {" + Invalid Procfile format + + The provided `Procfile` contains an invalid format and the buildpack cannot continue. + + To fix this problem please correct the following error and commit the results to git: + + {parsing_error} + "}), ProcfileBuildpackError::ProcfileConversionError(conversion_error) => match conversion_error { ProcfileConversionError::InvalidProcessType(libcnb_error) => { diff --git a/src/main.rs b/src/main.rs index 6597869..8dde19a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -43,12 +43,13 @@ impl Buildpack for ProcfileBuildpack { .map_err(ProcfileBuildpackError::ProcfileParsingError) })?; - if procfile.is_empty() { - bullet = bullet.sub_bullet("Empty file, no processes defined"); - } else { - for (name, command) in &procfile.processes { - bullet = bullet.sub_bullet(format!("{name}: {}", style::command(command))); - } + let warning_prefix = style::important("WARNING:"); + for message in &procfile.warnings { + bullet = bullet.sub_bullet(format!("{warning_prefix} {message}")); + } + + for (name, command) in &procfile.processes { + bullet = bullet.sub_bullet(format!("{name}: {cmd}", cmd = style::command(command))); } bullet.done().done(); diff --git a/src/procfile.rs b/src/procfile.rs index cd75ee6..f379f9d 100644 --- a/src/procfile.rs +++ b/src/procfile.rs @@ -1,111 +1,512 @@ +//! Contains logic for parsing the `Procfile` format +use bullet_stream::style; use linked_hash_map::LinkedHashMap; -use regex::Regex; -use std::str::FromStr; +use std::fmt::Display; +use winnow::{ + ascii::{line_ending, space0, till_line_ending}, + combinator::{alt, eof, opt, preceded, repeat, repeat_till, terminated, trace}, + error::{ContextError, ParseError, StrContext, StrContextValue}, + prelude::*, + stream::{Offset, Stream}, + token::{one_of, take_while}, + Parser, +}; #[derive(Debug, Eq, PartialEq)] pub(crate) struct Procfile { pub(crate) processes: LinkedHashMap, + pub(crate) warnings: Vec, } impl Procfile { + #[cfg(test)] pub(crate) fn new() -> Self { - Procfile { + Self { processes: LinkedHashMap::new(), + warnings: Vec::new(), } } - pub(crate) fn is_empty(&self) -> bool { - self.processes.is_empty() - } - #[cfg(test)] - pub(crate) fn insert(&mut self, k: impl Into, v: impl Into) { - self.processes.insert(k.into(), v.into()); + pub(crate) fn insert(&mut self, key: &str, value: &str) { + self.processes.insert(key.to_string(), value.to_string()); } } -impl Default for Procfile { - fn default() -> Self { - Procfile::new() +#[derive(Debug)] +pub(crate) enum ProcfileError { + ParseError(ProcfileParseError), +} + +impl Display for ProcfileError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ProcfileError::ParseError(error) => write!(f, "{error}"), + } } } -impl FromStr for Procfile { - type Err = ProcfileParsingError; +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct ProcfileParseError { + message: String, + span: std::ops::Range, + input: String, +} + +impl ProcfileParseError { + fn from_parse(error: &ParseError<&str, ContextError>, input: &str) -> Self { + let message = error.inner().to_string(); + let input = input.to_owned(); + let start = error.offset(); + // Assume the error span is only for the first `char`. + // Semantic errors are free to choose the entire span returned by `Parser::with_span`. + let end = (start + 1..input.len()) + .find(|e| input.is_char_boundary(*e)) + .unwrap_or(start); - fn from_str(procfile_contents: &str) -> Result { - let re_carriage_return_newline = Regex::new("\\r\\n?").expect("Clippy checked"); - let re_multiple_newline = Regex::new("\\n*\\z").expect("Clippy checked"); + Self { + message, + span: start..end, + input, + } + } +} - // https://github.com/heroku/codon/blob/2613554383cb298076b4a722f4a1aa982ad757e6/lib/slug_compiler/slug.rb#L538-L545 - let re_procfile_entry = Regex::new("^[[:space:]]*([a-zA-Z0-9_-]+):?\\s+(.*)[[:space:]]*") - .expect("Clippy checked"); +impl std::str::FromStr for Procfile { + type Err = ProcfileError; - let procfile_contents = re_carriage_return_newline.replace_all(procfile_contents, "\n"); - let procfile_contents = re_multiple_newline.replace(&procfile_contents, "\n"); + fn from_str(input: &str) -> Result { + let (processes, mut warnings) = parse_procfile + .parse(input) + .map_err(|e| ProcfileError::ParseError(ProcfileParseError::from_parse(&e, input)))?; + + if processes.is_empty() { + warnings.push("Empty file, no processes defined".to_string()); + } Ok(Procfile { - processes: procfile_contents - .lines() - .filter_map(|line| re_procfile_entry.captures(line)) - .filter_map(|cap| { - cap.get(1).and_then(|name| { - cap.get(2).map(|command| { - (String::from(name.as_str()), String::from(command.as_str())) - }) - }) - }) - .collect::>(), + processes, + warnings, }) } } -// There are currently no ways in which parsing can fail, however we will add some in the future: -// https://github.com/heroku/buildpacks-procfile/issues/73 -#[derive(Debug, Eq, PartialEq)] -pub(crate) enum ProcfileParsingError {} +impl std::fmt::Display for ProcfileParseError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let message = annotate_snippets::Level::Error + .title(&self.message) + .snippet( + annotate_snippets::Snippet::source(&self.input) + .fold(true) + .annotation(annotate_snippets::Level::Error.span(self.span.clone())), + ); + let renderer = annotate_snippets::Renderer::plain(); + let result = renderer.render(message); + result.fmt(f) + } +} + +/// Returns a mapping of key/values and warnings from a Procfile +fn parse_procfile(input: &mut &str) -> PResult<(LinkedHashMap, Vec)> { + let mut warnings: Vec = Vec::new(); + let mut key_values: Vec<(String, String)> = Vec::new(); + let mut out = LinkedHashMap::new(); + + while !input.is_empty() { + opt(parse_ignored_lines).parse_next(input)?; + + let checkpoint = input.checkpoint(); + // Strict path + if let Ok(kv) = parse_key_value(input) { + key_values.push(kv); + } else { + input.reset(&checkpoint); + match parse_permissive_key_fixed(input) { + Ok((original, fixed)) => { + let value = parse_value.parse_next(input)?; + + warnings.push(format!( + "Procfile key {} has been corrected to {}. Please update your Procfile.", + style::value(&original), + style::value(&fixed) + )); + key_values.push((fixed, value)); + } + Err(err) => { + return Err(err); + } + } + } + + opt(parse_ignored_lines).parse_next(input)?; + } + + for (key, value) in key_values { + if out.contains_key(&key) { + warnings.push(format!( + "Duplicate key `{key}` found. The value `{value}` will be used." + )); + } + out.insert(key, value); + } + + Ok((out, warnings)) +} + +/// Extracts and transforms a semi-valid key or returns an error +/// +/// Semi-valid key transformations +/// - Remove spaces at the start +/// - Transform `_` to `-` +/// - Transform uppercase to lowercase characters +/// +/// Any other values will be invalid. +/// +/// Returns (original, fixed) tuple on success +fn parse_permissive_key_fixed(input: &mut &str) -> PResult<(String, String)> { + let checkpoint = input.checkpoint(); + let original = parse_permissive_key(input)?; + + let fixed_input_string = format!("{}:", original.replace('_', "-").to_ascii_lowercase()); + let mut fixed_input = fixed_input_string.as_str(); + let before_strict = fixed_input.checkpoint(); + + // Remove leading spaces + opt(space0::<&str, ContextError<&str>>) + .parse_next(&mut fixed_input) + .expect("opt cannot fail"); + parse_key + .parse_next(&mut fixed_input) + .map(|fixed| (original.to_string(), fixed)) + .inspect_err(|_| { + // In the event of an error, reset the input and determine what character caused the unrecoverable error + input.reset(&checkpoint); + // Determine how far the input moved before an error was encountered (in number of tokens) + let error_offset = fixed_input.checkpoint().offset_from(&before_strict); + // Advance the original input that number of tokens + for _ in 0..error_offset { + input.next_token(); + } + }) +} + +/// A strictly validated single `key: value` pair in a `Procfile` +fn parse_key_value(input: &mut &str) -> PResult<(String, String)> { + let key: String = parse_key + .context(StrContext::Label("key")) + .parse_next(input)?; + let val = parse_value.parse_next(input)?; + + Ok((key, val)) +} + +/// Takes all characters terminated by `:` +/// +/// Used for transforming `_` to `-` and emitting warnings +fn parse_permissive_key<'s>(input: &mut &'s str) -> PResult<&'s str> { + terminated(take_while(0.., |c| c != ':'), ':').parse_next(input) +} + +/// Pattern represents a strict Procfile key +/// +/// +/// - Must start and end with a lowercase alphanumeric value ('a'..='z' or '0'..='9') +/// - Inner characters must be lowercase alphanumeric or a dash `-` (underscore `_` and other delimiters are not allowed) +fn parse_key(input: &mut &str) -> PResult { + alt(( + terminated(parse_lower_alphanum1, ':').map(|key| key.to_string()), + parse_two_or_more_char_key, + )) + .verify(|key: &str| key.chars().count() <= 63) + .context(StrContext::Expected(StrContextValue::Description( + "keys contain characters or fewer", + ))) + .parse_next(input) +} + +/// Value part of a `key: value` entry in procfile +fn parse_value(input: &mut &str) -> PResult { + preceded(space0, till_newline_or_eof) + .verify(|value: &str| !value.is_empty()) + .context(StrContext::Label("value")) + .map(std::string::ToString::to_string) + .parse_next(input) +} + +/// Lowercase alphanumeric value +fn parse_lower_alphanum1(input: &mut &str) -> PResult { + alt((one_of('a'..='z'), one_of('0'..='9'))) + .context(StrContext::Expected(StrContextValue::Description( + "lowercase alphanumeric value (a-z0-9)", + ))) + .parse_next(input) +} + +/// Returns a key from `key: value` pair that has two or more characters +/// +/// First and last character must be lowercase alphanumeric (a-z0-9) +/// Middle characters can be lowercase alphanumeric or `-` +fn parse_two_or_more_char_key(input: &mut &str) -> PResult { + ( + parse_lower_alphanum1.context(StrContext::Label("first key character")), + parse_middle_tail_key_chars, + ) + .map(|(start, tail)| { + let mut key = String::new(); + key.push(start); + key.push_str(&tail); + key + }) + .parse_next(input) +} + +/// Parses middle characters followed by a valid ending character +fn parse_middle_tail_key_chars(input: &mut &str) -> PResult { + // The `repeat_till` will check the terminator matches before consuming + // the first parser, this is needed because the ending character is a subset + // of middle characters + repeat_till( + 0.., + alt((parse_lower_alphanum1, '-')) + .context(StrContext::Label("inner key character")) + .context(StrContext::Expected(StrContextValue::Description( + "lowercase alphanum (a-z0-9) or `-`", + ))), + ( + parse_lower_alphanum1.context(StrContext::Label("last key character")), + ':'.context(StrContext::Label("key delimiter")) + .context(StrContext::Expected(StrContextValue::CharLiteral(':'))), + ) + .map(|(last, _delimiter)| last), + ) + .map(|(middle, last): (String, char)| { + // + let mut tail = String::new(); + tail.push_str(&middle); + tail.push(last); + tail + }) + .parse_next(input) +} + +/// Returns all characters up to (but not including) the newline or EOF +fn till_newline_or_eof<'s>(input: &mut &'s str) -> PResult<&'s str> { + terminated(till_line_ending, alt((line_ending, eof))).parse_next(input) +} + +/// Consume comments and empty lines +fn parse_ignored_lines(input: &mut &'_ str) -> PResult<()> { + trace( + "ignored lines", + repeat(1.., alt((parse_comment, (terminated(space0, line_ending))))), + ) + .parse_next(input) +} + +/// A comment line in a Procfile +/// +/// Starts with `#` optionally preceded with spaces +fn parse_comment<'s>(input: &mut &'s str) -> PResult<&'s str> { + preceded(space0, preceded("#", till_line_ending)).parse_next(input) +} #[cfg(test)] mod tests { use super::*; + use bullet_stream::strip_ansi; + use libcnb_test::assert_contains; + + #[test] + fn test_parse_lower_alphanum1() { + assert!(parse_lower_alphanum1.parse(" ").is_err()); + } + + #[test] + fn test_replacing_upper_with_lowercase_warning() { + let input = "IamAvalidKeyButNotStrictly: echo 'done'"; + let result: Procfile = input.parse().unwrap(); + assert_eq!(1, result.warnings.len()); + assert_eq!("Procfile key `IamAvalidKeyButNotStrictly` has been corrected to `iamavalidkeybutnotstrictly`. Please update your Procfile.".to_string(), strip_ansi(result.warnings.last().unwrap())); + assert_eq!( + "echo 'done'", + result.processes.get("iamavalidkeybutnotstrictly").unwrap() + ); + } + + #[test] + fn test_invalid_start() { + let input = "-key: echo 'done'"; + assert!(input.parse::().is_err()); + } + + #[test] + fn test_invalid_end() { + let input = "key-: echo 'done'"; + assert!(input.parse::().is_err()); + } + + #[test] + fn test_one_char_key() { + let input = "a: echo 'done'"; + input.parse::().unwrap(); + } + + #[test] + fn test_two_char_key() { + let input = "aa: echo 'done'"; + input.parse::().unwrap(); + } + + #[test] + fn test_strictly_valid_one_key_val_combo() { + let input = "iamastrictly-validkey: echo 'done'"; + let result: Procfile = input.parse().unwrap(); + assert_eq!(0, result.warnings.len()); + assert_eq!( + "echo 'done'", + result.processes.get("iamastrictly-validkey").unwrap() + ); + } + + #[test] + fn test_till_line_ending() { + let mut input = "a\nb\nc"; + let result = till_newline_or_eof(&mut input).unwrap(); + assert_eq!("a", result); + assert_eq!("b\nc", input); + let result = till_newline_or_eof(&mut input).unwrap(); + assert_eq!("b", result); + let result = till_newline_or_eof(&mut input).unwrap(); + assert_eq!("c", result); + + let mut input = " a\n b\n c"; + let result = till_newline_or_eof(&mut input).unwrap(); + + assert_eq!(" a", result); + } + + #[test] + fn comment_test() { + let mut input = "# I ama comment"; + let comment = parse_comment(&mut input).unwrap(); + assert_eq!(" I ama comment", comment); + + let mut input = " # Comment with spaces"; + let comment = parse_comment(&mut input).unwrap(); + assert_eq!(" Comment with spaces", comment); + + let mut input = "I am not a comment # <--"; + let result = parse_comment(&mut input); + assert!(result.is_err()); + } + + #[test] + fn process_key_value() { + let (key, val) = parse_key_value.parse("web: rails s").unwrap(); + assert_eq!("web", key); + assert_eq!("rails s", val); + } #[test] fn test_empty_parse_procfile() { - assert_eq!("".parse::(), Ok(Procfile::new())); + let procfile = "".parse::().unwrap(); + assert_eq!(0, procfile.processes.len()); + assert_eq!( + &"Empty file, no processes defined".to_string(), + procfile.warnings.first().unwrap() + ); + assert_eq!(1, procfile.warnings.len()); } #[test] fn test_valid_parse_procfile() { let mut expected_procfile = Procfile::new(); - expected_procfile.insert("web", "rails s"); + expected_procfile + .processes + .insert("web".to_string(), "rails s".to_string()); - assert_eq!("web: rails s".parse::(), Ok(expected_procfile)); + assert_eq!( + expected_procfile, + "web: rails s".parse::().unwrap() + ); } #[test] fn test_multiple_valid_parse_procfile() { let mut expected_procfile = Procfile::new(); - expected_procfile.insert("web", "rails s"); - expected_procfile.insert("worker", "rake sidekiq"); + expected_procfile + .processes + .insert("web".to_string(), "rails s".to_string()); + expected_procfile + .processes + .insert("worker".to_string(), "rake sidekiq".to_string()); assert_eq!( - "web: rails s\nworker: rake sidekiq".parse::(), - Ok(expected_procfile) + expected_procfile, + "web: rails s\nworker: rake sidekiq" + .parse::() + .unwrap(), ); } #[test] fn test_nonsense_procfile() { - assert_eq!("&&&&&".parse::(), Ok(Procfile::new())); + assert!("&&&&&".parse::().is_err()); } #[test] fn test_missing_command_parse_procfile() { - assert_eq!("web:".parse::(), Ok(Procfile::new())); + assert!("web:".parse::().is_err()); } #[test] fn test_missing_name_parse_procfile() { - assert_eq!(": rails -s".parse::(), Ok(Procfile::new())); + assert!(": rails -s".parse::().is_err()); + } + + #[test] + fn not_yaml_format_but_still_valid() { + let input = r" +# Comment + + web: echo foo: bar +"; + let procfile = input.parse::().unwrap(); + assert_eq!(1, procfile.warnings.len()); + } + + #[test] + fn invalid_procfile_key_points_at_the_correct_location_of_the_fatal_error() { + let input = "is_w.e.b: echo hello"; + let result = input.parse::(); + assert!(result.is_err()); + match result { + Ok(_) => panic!("Expected error, got {result:?}"), + Err(e) => assert_contains!( + &format!("{e}").trim(), + r" +1 | is_w.e.b: echo hello + | ^ +" + .trim() + ), + } + } + + #[test] + fn max_length_key_is_63_chars() { + let input = r" +# 63 chars ---------------------------------------------------v +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: fonz +"; + let result = input.parse::(); + assert!(result.is_ok()); + + let input = r" +# 64 chars ----------------------------------------------------v +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: fonz +"; + let result = input.parse::(); + assert!(result.is_err()); } } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 99d5614..7a3eb0d 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -154,6 +154,7 @@ fn test_not_yaml_procfile() { ## Procfile Buildpack - Processes from `Procfile` + - WARNING: Procfile key ` web` has been corrected to `web`. Please update your Procfile. - web: `echo foo: bar ` - Done (finished in < 0.1s) "} @@ -179,7 +180,7 @@ fn test_empty_procfile() { ## Procfile Buildpack - Processes from `Procfile` - - Empty file, no processes defined + - WARNING: Empty file, no processes defined - Done (finished in < 0.1s) "} );