Repository navigation
Prerelease candiate for v1.0.0A8 - #14
Closed
Static-Codes wants to merge 858 commits into
Closed
Static-Codes wants to merge 858 commits into
Static-Codes wants to merge 858 commits into
Conversation
…ded and extracted.
…ompilation status for Debian/Fedora/Standalone
…e cross compatability
…anagers.AppManager.OS.Linux.Functions.GetApps()
Documented future plans to rewrite BAMM to be less verbose and easier to maintain. This new rewrite will up the major version to 2.
Documented future release plans (following a rewrite)
Updated license to GPLv3 and added license notice to add source files
Add demo tag
add: gui instructions to quickstart guide in readme
* Fixed validation bug for commands with two quoted args * Readded a crucial debug function in the DSL parser.
This PR addressed #10, which laid out the requirements for these unit test implementations.
BAMM has been licensed under GPLv3 for over 3 months, however, there have been no releases in roughly 8 months, so this was missed in the process. Commit: ec205c3
Brings the GPLv3 relicense, the parser validation fix (#11), and the xUnit test suite (#12, #13) onto canary. Conflict resolution: 43 content conflicts resolved in favor of canary's Core/ namespace layout, carrying over stable's GPLv3 headers. The src/Tests native-AOT harness and 11 superseded Managers/ files are removed in favor of their Core/ replacements. Solution file repointed from Tests/ to BrowserAutomationMaster.Tests/. Known issue: 8 of the 59 newly added tests fail. Verified identical on a pristine stable worktree, so the merge introduces none of them. - FeatureTests (5): tests v1 syntax (`--disable-ssl`); the parser expects `feature "disable-ssl"`. The default switch arm correctly returns false. - JavaScriptBlockTests (1): start-javascript/end-javascript are handled in the file-level loop (Parser.cs:655), not in HandleLineValidation, so calling it directly cannot return true. - ClickTests (1): the XPATH case has spaces inside the quotes, so line.Split(" ") yields 3 parts, failing the lineArgs.Length != 2 check at LineValidation.cs:140. - KnownBugTests (1): a known-bug guard asserting add-header throws in a headless environment; it now returns a validation false instead. No positive assertion, so it cannot distinguish fixed from unreachable. Introduced in a4dc873. Safe to delete.
…ling of a merge conflict during #15.
The suite had 8 failing tests once the merge landed. None of them were caused by the merge itself, but all of them had to be sorted out before the suite could be pushed. In the process a bug in add-header validation was discovered and rectified. The XUnit suite came across from stable in the previous merge with 8 tests already failing. None of those failures were down to the merge, but they had to be cleared up before the suite could be relied on. Three separate causes turned up, and all three were already sitting on stable. 1. Platform detection was never getting initialized (21 failures) Every test that reached DirectoryManager or Parser threw PlatformNotSupportedException. What tripped it up was ordering. ProgramFunctions.InitializeAsync calls PlatformManager.SetPlatform, which sets the PlatformInfo flags that GetAppDataDirectory branches on. The tests call Parser directly, so that call never happened, the flags stayed false, and the method dropped through to its throw. PlatformInitializer.cs is a new module initializer that makes the same call once before any test in the assembly runs. A module initializer was chosen over an xUnit IAssemblyFixture because the existing tests take no fixtures, so each one would have needed to opt in. That took the count from 21 failures down to 8. 2. Tests were asserting things the parser never did (7 failures) These were bugs in the tests themselves, and in each case a test had been written against an assumption that never held. FeatureTests asserted the v1 syntax (--disable-ssl) and expected true. The parser has only ever accepted feature "disable-ssl". Searching the whole project history turns up no sign of the v1 form. These were converted to v2 and the file grew from 6 cases to 11, including --disable-ssl as an explicit negative so the removal stays a deliberate decision rather than an accident. The feature grammar in 574dca9 and the forms already in use in IntegrationTests were both used as the reference. Three test files called HandleLineValidation(line, "test.bamc", 1) against a signature of (fileName, line, lineNumber). They were validating the literal string "test.bamc" instead of their intended input. The argument order was corrected in FeatureTests, JavaScriptBlockTests and KnownBugTests. ClickTests asserted that click "//button[contains(text(), 'Submit')]" is valid. click splits on a bare space, so a selector can never contain one, and spaced selectors are only reachable through click-exp. The case had been misfiled. Coverage moved to ClickExpTests, plus a new case asserting that click rejects spaced selectors. 3. KnownBugTests was tracking a bug that does not exist The test claimed add-header throws IOException or ArgumentOutOfRangeException out of Console.WindowWidth in headless CI. Two checks say this never happened. For a start, Console.WindowWidth returns 0 when no tty is attached. It does not throw, verified directly. On top of that, the test called HandleLineValidation with its arguments swapped, so it was validating "test.bamc" and never reached add-header at all. It failed for an ordinary reason, and the headless crash reads like an explanation reached for afterwards. The test also could not tell "crash" from "fixed", since it was willing to accept either outcome. It was deleted along with the KnownBugs directory, which held nothing else. The same story had made it into the test README, sitting next to a /KnownIssues path that does not exist, /FormatValidators in place of /FormatValidation, and the v1 feature syntax. All corrected. Worth flagging: that test was initially rewritten without first checking whether the failure it described could even happen. A grep turned up Console.WindowWidth at ANSI.cs:263, that was taken as proof, and the made-up reasoning then carried straight into the rewrite. Parser fix: add-header could never validate The rewritten add-header tests turned this up. add-header sits in lineArgSpecialCases, so its arguments get split on ' "', which eats the opening quote on each one: 'add-header "Authorization" "Bearer token"'.Split(' "') -> ['add-header', 'Authorization"', 'Bearer token"'] AddHeader hands off to ValidateTwoArgCommand, which wants IsArgQuoted to find a quote at both ends. The opening quote has already gone by that point, so the check failed every single time and add-header turned down its own documented syntax. add-header was the only command hit here. fill-text, fill-text-exp and set-custom-useragent check EndsWith('"') themselves, and add-cookie and click-at-position stay out of the special-case list so they split on a bare space and keep both quotes. This came from 6eb460e back on 2026-01-17, "Added new action command add-cookie". That commit pushed add-header onto the shared helper, and the inline check it replaced was EndsWith-only, so the command worked right up until the delegation changed. The pre-refactor code in Parser.cs is fine, and 6eb460e only swapped the call. There is a second layer too. 4eff309 (#11) added the exact parameter needed to put this right, documenting it as "If the argument had a leading double quote removed prior to validation". No caller ever passed it. The merge in 8d4c52a then settled that hunk in canary's favor and dropped the parameter, leaving canary with none of #11's fix. It was restored, and AddHeader now passes stripped: true. That fix went into the previous commit (22b1355) and is covered by the new Commands/AddHeaderTests.cs, which includes a case that reaches the quote check rather than the argument count, so the validation cannot be quietly loosened later. Verification 73 passing, 0 failing, and the solution builds clean with no warnings. That second one passed on the first attempt, which was worth catching. Every case in AddHeader_RejectsMalformedInput was getting turned away by the argument-count check before it ever reached the quote logic, so they were asserting nothing at all. The case add-header "Authorization" "token" extra was added, which gives three arguments with trailing content on the second one, so it lands on the quote check where it matters. Still open A selector with nested quotes, //button[contains(text(), 'Submit')], cannot be written with either command. click breaks on the space and click-exp breaks on the inner quotes, and neither one supports escaping. Noted as a known limitation in ClickExpTests. feature "add-extension" with a trailing space gets accepted, since the second argument of a non-proxy feature never gets looked at. Noted in FeatureTests. AppLicenseType in src/Publisher/Build/BuildInfo.cs still reads "MIT", so the Arch and Debian packages will declare MIT while the project itself is GPLv3. That is already present on stable and was not introduced here, though it does ship in the same stretch as the relicense.
Every existing test calls a function directly. Commands, Features, FormatValidation, Selectors and Integration all pass strings to Parser or its helpers and assert on the boolean that comes back. None of them ever hands a .bamc file to the compiler, which is the only part of BAMM a user runs end to end. The new ScriptExecution suite does. It enumerates the .bamc scripts it can find, runs the real CLI as a child process with compile <file>, and asserts on the exit code. On its first run it reported 8 defects, and 2 of the 11 example scripts in the repository had never compiled. 4 of the 8 share 1 cause: the compiler re-implements parsing instead of calling Parser.IsValidFile, so the 2 implementations have drifted apart. The parser enforces a feature allowlist, command arity and quote rules that the compiler never consults, and a script the parser rejects still compiles. The worst of those 4 is silent. Give the compiler a command it does not recognise and it emits nothing for that line and exits zero. A script that quietly drops a step is more dangerous than one that refuses to run, because the run looks successful. The fix could not live in the emit switch. Most of its cases are written case "click" when Click(...) is false, so the case matches only when the command fails. A command that succeeds matches nothing and falls out of the switch, and a command that was never recognised does exactly the same thing. The compiler cannot distinguish the 2 from inside that switch, so the check has to happen before it. Adding a default arm instead broke all 11 examples, which is what exposed the problem. The remaining 3 in that group are smaller. Parser.IsValidFile set featureBlockFinished from the first non-comment line it saw, so the first feature line closed the feature block and a second one was reported as misplaced; a script could declare exactly 1 feature. GetDesiredUrls splits raw lines on a space and requires exactly 2 tokens, so a visit command carrying a trailing comment was not collected and the script came back reporting no visit commands at all. DeleteCommentIfPresent searches for " // ", so a comment consisting of 2 slashes and nothing else was never removed. The example scripts were part of the same problem. no-ssl-example.bamc is the only script in the repository containing a feature command, so Parser.featureArgs had no example exercising it, and that script used feature "no-ssl", which is not in the allowlist, with both feature lines placed after the visit. Fixing the compiler alone would have left the sole example for the feature block still broken. 2 of the 8 are outside the compiler. GetTerminalBackgroundColor is the one that escapes this repository. It runs printf with an OSC 11 escape to /dev/tty and reads the reply with read from /dev/tty. /dev/tty is the controlling terminal, the same one the invoking shell is reading from, so any BAMM run with a redirected stdin left the terminal response in the shell input queue, where the next command typed at that terminal consumed it. The same code path also emitted an exception on every such run, because ForegroundMatch parses a single rgb group and the queued responses concatenate. StartGUIThread has a default parameter of DEFAULT_PORT, and the call site that parses --gui --port==X passed no argument at all, so the listener bound 8008 regardless of the port the user passed. The line above it still carries a commented-out StartServer(port), which dates the regression. Out-of-range values now get a message instead of reaching HttpListener. The GUI server answered an unrecognised route by logging a warning and falling out of the switch without writing a response, so the socket stayed open with nothing sent and the browser saw a network error with no status code. It now returns 404 with the reason in the body. Also added, not fixed: a /gui_version endpoint reporting the version of the GUI this build embeds. The GUI moved to its own repository and its own release cadence, so the version is read back out of the embedded archive rather than from a constant that could drift from the archive. Making the suite usable needed 2 changes. BAMM writes through Spectre.Console to the process-global Console from inside its validation routines, and xunit runs 1 collection per test class in parallel, so the output interleaved character by character. xunit.runner.json disables collection parallelism. ConsoleCapture additionally redirects that output into a buffer, because the inverse tests print every rejection they provoke, which left a passing run looking like a wall of errors. Verification: 110 passing, 0 failing, 1 skipped. 10 of the 11 examples compile; marketplace.bamc does not, because facebook.com returns HTTP 400 to the Firefox User-Agent that RequestManager sends and 200 to the curl default, so the compiler and the pre-flight probe agree the URL is unreachable. The GUI server was exercised over HTTP rather than mocked: /gui_version returns the expected version on the default port and on a requested one, an unknown route returns 404, and /load still returns 200. Left undone deliberately. Transpiler.validCommands is hand-maintained because the switch cannot detect unknown commands, and CompilationValidationTests asserts it matches Parser.HandleLineValidation exactly so the 2 cannot drift apart silently; removing the duplicated parsing entirely is a larger change than this. A selector containing its own quotes remains unwritable through either click or click-exp, as ClickExpTests already records. sections/changelog-for-next.md is updated throughout, including the provenance of 2 unrelated cleanups that were already in the working tree when this changeset was assembled.
BAMM identified its host distribution with a hand rolled /etc/os-release parser and a hand maintained package manager table, where each of the 15 entries in Distros.cs restated the same handful of values. That table had drifted. PCLinuxOS reported an apt package manager but was published as an rpm, and Oracle Linux never matched because the real ID is ol and not oracle. WhichDistroSharp 1.0.1 owns both of those now. Distros.cs keeps only what is unique to BAMM, and each entry names the upstream distros it claims rather than a single ID. Two types collided on import, so the upstream enum is aliased to WhichDistro and the unambiguous names moved into GlobalUsings.cs. Detection resolves a dictro in O(1) from a dictro to entry table, and the 93 distros BAMM cannot identify are an explicit and tested contract rather than a silent prompt on every run. The Linux installer reads a generated package format map covering 110 distros in place of 12 hardcoded ones, and falls back to those when the fetch fails so an offline host still installs. Publishing the map is a build target, since a test cannot both assert against a snapshot and rewrite it. Nine defects were fixed along the way, and the two that changed user visible behaviour were these. 1. Raspbian, openSUSE Leap and openSUSE Tumbleweed were never matched Their real ID values matched no entry, so those hosts were asked to pick a base distribution on every single run. install.sh has always treated Raspbian as Debian family, so the installer and the application disagreed about it. 2. The Linux installer's error guards did nothing show_error_and_exit is called three times near the top of install.sh and defined near the bottom. Bash resolves a function when it is called and not when the file is read, so every one of those calls reported command not found and the script carried on. A macOS user was told nothing and the script went on to install Linux packages. 3. The installer could install a package left over from a failed run wget derives the file name from the URL and saves to <name>.1 when the target already exists. The temporary directory is only removed on success, so the next run installed the stale package instead of the new release. The download now uses wget -O, which forces the name. 4. RefreshDebianAptCache never ran It compared the Distro object against a DistroBase value, so it was always false and apt-get update never executed, including on the packaging paths that call it because a dependency is missing. The rename alone would not have fixed it, since object.Equals accepts anything and returns false across types. 5. Python 3.13 and 3.14 could not be selected HandlePythonVersionSelection filled a fixed string[6] against a mapping of eight. Installing six interpreters dropped the last two. 6. CI tested a different repository than the one under review dotnet.yml checked out the triggering commit, then cloned the default branch and built that instead. The trigger also named main, which is not a branch in this repository, and GitHub resolves a missing ref to whatever it last pointed at, so the workflow had not run since January. 7. install.sh did not recognise Oracle Linux The ID extraction was also unanchored, so ID_LIKE could match ahead of ID, and it left the quotes in ID=ubuntu behind. 8. Terminal probe tests failed only on a workstation The suite asserted its own precondition with Assert.True, which holds under CI and is false for anyone running dotnet test from an interactive terminal. It now skips. 9. Unsupported distribution messages pointed at the wrong problem A distro BAMM ships no package for was reported as not currently supported, which is not something a user can act on. Coverage went with it. DistroMappingTests pins every forwarded value to the literal that was in Distros.cs before the move, so a refactor that changes what BAMM executes fails the suite rather than on a machine. test-install.sh covers install.sh, which is curl | bash and cannot be reached by xunit at all, and asserts the helpers are defined before they are first used. The 93 unclaimed distros and the 4 published package formats are recorded in sections/known-distribution-gaps.md, which is linked from the README.
Owner
Author
|
After a couple days of trial and error, I was finally able to replace BAMM's fragile distro and package resolution logic using a combination of WhichDistroSharp, source generation, and an MSBuild target that generates and ships an updated test file. The aforementioned test file properly identifies blindspots in distro coverage, allowing for both end-users and developers to stay updated on BAMM's linux support! Commit: 40b0af8 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This build needs significant testing before release, namely around the NFD support (on osx-arm64).
For more information see this extremely detailed changelog: https://github.com/Static-Codes/BrowserAutomationMaster/blob/815f557d65a871bb6e6894d76aa3b3a60e46f107/sections/changelog-for-next.md
Eventually, the distro detection logic should be reworked to use
WhichDistroSharp.That said, it is currently 2:30AM and I need sleep.