projects/love_fipamo
projects/love_fipamo
1
0
Fork 1
Code Issues 5 Pull requests 1 Projects Releases Packages Wiki Activity Actions

WIP: tests for Page roundtripping and the start of a CI pipeline #11

Draft
federico wants to merge 21 commits from federico/love_fipamo:book-tests into develop
pull from: federico/love_fipamo:book-tests
merge into: projects:develop
Conversation 5 Commits 21 Files changed 9 +311 -152
federico commented 2025-10-14 02:29:40 +02:00
Collaborator
Copy link

This is a work in progress:

  • A little refactoring to allow having a roundtrip test for Page -> markdown -> Page. This is to ensure that we can refactor the pages code, while being confident that we don't lose data when saving and re-loading from disk.

  • Start writing a CI pipeline. I'm not sure all the Forgejo machinery is set up yet; the pipeline doesn't run but yields no errors?

What I really want to do after the roundtripping test:

  • Make Page either store a Frontmatter struct instead of its fields, or have a better separation between fields that are for storage and fields that are for display purposes (maybe the latter can be computed on the fly for HTTP responses).

  • Generate a page's markdown by serializing that Frontmatter to YAML instead of piecing together a string by hand.

This is a work in progress: * A little refactoring to allow having a roundtrip test for `Page` -> markdown -> `Page`. This is to ensure that we can refactor the pages code, while being confident that we don't lose data when saving and re-loading from disk. * Start writing a CI pipeline. I'm not sure all the Forgejo machinery is set up yet; the pipeline doesn't run but yields no errors? What I really want to do after the roundtripping test: * Make `Page` either store a `Frontmatter` struct instead of its fields, or have a better separation between fields that are for storage and fields that are for display purposes (maybe the latter can be computed on the fly for HTTP responses). * Generate a page's markdown by serializing that `Frontmatter` to YAML instead of piecing together a string by hand.
And avoid allocations, since we return &'static str instead of a String.
Book::get_page_with_uuid("0") was used to create a new empty page,
but:

  * it just returns Default::default() anyway
  * it does not insert the Page into the book
  * it does not look into the Book at all to set up some of the Page's
    properties

So, let's just let callers create the pages by themselves directly.
If we later need to let the Book set up a new page somehow (like
figuring out an unused UUID or something), we can add a
Book.create_empty_page() method.
We'll use this to write a Page <-> markdown roundtripping test.

I changed e.g.

  &page.raw_feature.unwrap().to_string()

to

  page.raw_feature.as_ref().unwrap()

to avoid having the former move out the contents of the raw_feature
field; instead the latter just extracts a &str without moving.
This doesn't test anything in particular other than the contents of
the markdown string are what the current code does.
It has no await points.
Reallocation is not a performance concern yet; let's just use fresh
arrays and then we don't have to clear them between iterations.
This is now the Page::parse_from_markdown() function.  Some notes:

* The Frontmatter struct is now in pages.rs; it seems appropriate,
  since it only pertains to the serialization of pages, and not to the
  book at large.

* parse_from_markdown() returns a Result, just Ok() for now, but we'll
  propagate the result from markdown_frontmatter shortly.

Again, this function is extracted to be able to write a
Page <-> markdown roundtripping test.
The test fails here:

---- tests::markdown::roundtrips_page_to_markdown stdout ----

thread 'tests::markdown::roundtrips_page_to_markdown' panicked at src/core/library/pages.rs:86:30:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/std/src/panicking.rs:697:5
   1: core::panicking::panic_fmt
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/core/src/panicking.rs:75:14
   2: core::panicking::panic
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/core/src/panicking.rs:145:5
   3: core::option::unwrap_failed
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/core/src/option.rs:2072:5
   4: core::option::Option<T>::unwrap
             at /home/federico/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/option.rs:1005:21
   5: love_fipamo::core::library::pages::Page::parse_from_markdown
             at ./src/core/library/pages.rs:86:30
   6: love_fipamo::tests::markdown::roundtrips_page_to_markdown
             at ./src/tests/markdown.rs:67:23
   7: love_fipamo::tests::markdown::roundtrips_page_to_markdown::{{closure}}
             at ./src/tests/markdown.rs:64:33
   8: core::ops::function::FnOnce::call_once
             at /home/federico/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
   9: core::ops::function::FnOnce::call_once
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/core/src/ops/function.rs:250:5

I.e. when building a MediaItem, there is no extension in the path.
Thsi is because the page that make_dummy_page() created just has this
in the frontmatter:

feature: raw_feature

and "raw_feature" does not split like a filename with an extension.
These are comma-separated lists of filenames with extension.  We'll
deal with validation of filenames later.
This pretty-prints failures in assert_eq!() calls; very useful to make
failed tests readable.
WIP: add a CI pipeline
Some checks failed
pipeline.yaml / WIP: add a CI pipeline (pull_request) Failing after 0s
Details
d72d6d4251
Fix roundtripping of pages without featured MediaItems
Some checks failed
pipeline.yaml / Fix roundtripping of pages without featured MediaItems (pull_request) Failing after 0s
Details
a1b41b5006 
The test indicates that we want a missing `feature` key in the
markdown frontmatter to be parsed as None.  Conversely, a None should
serialize as nothing.
federico commented 2025-10-18 04:54:30 +02:00
Author
Collaborator
Copy link

@ro Hey there! Any chance of setting up a CI runner so my pipeline can fail properly? :)

@ro Hey there! Any chance of setting up a CI runner so my pipeline can fail properly? :)
ro commented 2025-10-20 17:36:02 +02:00
Collaborator
Copy link

So the challenge with the runner is that I'm not sure where to set it up.

Apparently, it's frowned upon to set up on as the same server as the repo, so I'm not sure about implementation outside of setting up a new server just for this.

I'm open to any suggestion on how to get his up and running.

If you want to put something together in the meantime, that's cool too.

So the challenge with the runner is that I'm not sure where to set it up. Apparently, it's frowned upon to set up on as the same server as the repo, so I'm not sure about implementation outside of setting up a new server just for this. I'm open to any suggestion on how to get his up and running. If you want to put something together in the meantime, that's cool too.
federico commented 2025-10-28 00:44:46 +01:00
Author
Collaborator
Copy link

I can understand not wanting to put a runner on the same server as a repo, if you have a repo hosting platform open to the public.

FWIW, for my home server (gitea + drone, have not moved it to forgejo yet), I set up a docker runner on the same server. The only heavy CI pipelines there are run very sporadically, to rebuild my personal containers for development; most of the time the pipelines just deploy an internal website. The machine is old and slow and doesn't really bog down when a pipeline is active. (It's an old i7 with 32 GB of RAM, so old that the CPU doesn't even have AVX2 instructions... it runs fine.)

One nice thing about the docker runner is that the containers for jobs are really ephemeral and clean up after themselves. I don't have to run cleanup scripts for the CI or anything like that.

Since koodu is a more or less private server, I guess you can set up a runner there and trust that people won't use it for evil. Your call :)

I can understand not wanting to put a runner on the same server as a repo, if you have a repo hosting platform open to the public. FWIW, for my home server (gitea + drone, have not moved it to forgejo yet), I set up a docker runner on the same server. The only heavy CI pipelines there are run very sporadically, to rebuild my personal containers for development; most of the time the pipelines just deploy an internal website. The machine is old and slow and doesn't really bog down when a pipeline is active. (It's an old i7 with 32 GB of RAM, so old that the CPU doesn't even have AVX2 instructions... it runs fine.) One nice thing about the docker runner is that the containers for jobs are really ephemeral and clean up after themselves. I don't have to run cleanup scripts for the CI or anything like that. Since koodu is a more or less private server, I guess you can set up a runner there and trust that people won't use it for evil. Your call :)
federico commented 2025-10-28 01:45:11 +01:00
Author
Collaborator
Copy link

That said, I'll throw in the script I'm using while developing in lieu of CI - it's just

#!/bin/bash

set -eu -o pipefail

cargo build
cargo test
cargo fmt -- --check
cargo clippy
That said, I'll throw in the script I'm using while developing in lieu of CI - it's just ```sh #!/bin/bash set -eu -o pipefail cargo build cargo test cargo fmt -- --check cargo clippy ```
ro commented 2025-10-28 16:01:09 +01:00
Collaborator
Copy link

Hm, I appreciate the insight. Ha, not a fan of Docker, but its use case in this context makes sense.

It's something I need to learn how to set up anyway, so I'll read up and take the plunge so we can get it up and running.

The automation with testing and beyond is just too juicy to pass up.

Hm, I appreciate the insight. Ha, not a fan of Docker, but its use case in this context makes sense. It's something I need to learn how to set up anyway, so I'll read up and take the plunge so we can get it up and running. The automation with testing and beyond is just too juicy to pass up.
Some checks failed
pipeline.yaml / Fix roundtripping of pages without featured MediaItems (pull_request) Failing after 0s
Details
This pull request has changes conflicting with the target branch.
  • src/core/library/pages.rs
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u book-tests:federico-book-tests
git switch federico-book-tests

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch develop
git merge --no-ff federico-book-tests
git switch federico-book-tests
git rebase develop
git switch develop
git merge --ff-only federico-book-tests
git switch federico-book-tests
git rebase develop
git switch develop
git merge --no-ff federico-book-tests
git switch develop
git merge --squash federico-book-tests
git switch develop
git merge --ff-only federico-book-tests
git switch develop
git merge federico-book-tests
git push origin develop
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
projects/love_fipamo!11
No description provided.