Skip to content

Csanád's review of chapter 25 #373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 24, 2025
Merged

Csanád's review of chapter 25 #373

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5c9458c
fix typos
Xronophobe Apr 18, 2025
873d73c
add comments and suggestions
Xronophobe Apr 18, 2025
5cc9a53
maintain uniform styling of GitLab
Xronophobe Apr 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 50 additions & 6 deletions chapter_25_CI.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ They just don't need any _more_ help dominating the market.
// DAVID: 1e / 2e abbreviations might confuse people.

So I've decided to use GitLab in this book.
// CSANAD: I just found framagit.org by Framasoft. Maybe we could mention them? Although
// it might be important to ask them first, in case they need to handle the
// expected additional traffic.
They are absolutely a commercial service,
but they retain an open-source version, and you can self-host it in theory.

Expand Down Expand Up @@ -150,7 +153,7 @@ so the first thing to do is get our code up there.
Use the **New Project** -> **Create Blank Project** option, as in <<gitlab-new-blank-project>>.

//RITA: The screenshot shows that the project name and slug are "Superlistz", yet the next figure that shows the CI project files on GitLab refers to "lists". Is this correct?
.Creating a New Repo on Gitlab
.Creating a New Repo on GitLab
[[gitlab-new-blank-project]]
image::images/gitlab_new_blank_project.png["New Blank Project"]

Expand Down Expand Up @@ -383,7 +386,7 @@ so I know what to expect: it's failing because Firefox isn't installed
in the image we're using.


Let's modify the script, and an `apt install`.
Let's modify the script, and add an `apt install`.
Again we'll do it as late as possible.

[role="sourcecode"]
Expand Down Expand Up @@ -717,6 +720,10 @@ status: 0

It takes quite a long time to run,
and there's lots of debug out, but... it looks OK!
// CSANAD: I think it ran quite fast, less than maybe 30 seconds. I was confused
// whether it even worked correctly. I think we should maybe emphasize
// when it would "look OK": exit status being 0 and 200 OK, I assume?


Let's set that environment variable in our CI script:

Expand Down Expand Up @@ -777,6 +784,9 @@ AssertionError: '2: Use peacock feathers to make a fly' not found in ['1: Buy
peacock feathers']
---------------------------------------------------------------------
----
// CSANAD: we are running the FT-s locally now. I think it would be
// clearer if we said that explicitly and also if we said a few words on why we
// are running the FT without docker again.


Now you might not see this error,
Expand Down Expand Up @@ -843,7 +853,8 @@ SCREEN_DUMP_LOCATION = Path(__file__).absolute().parent / "screendumps"

We first create a directory for our screenshots if necessary.
Then we iterate through all the open browser tabs and pages,
and use a Selenium methods, `get_screenshot_as_file()`
// CSANAD: where? I can't find the iteration through the tabs and pages.
and use Selenium methods, `get_screenshot_as_file()`
and the attribute `browser.page_source`,
for our image and HTML dumps, respectively:

Expand All @@ -863,6 +874,11 @@ for our image and HTML dumps, respectively:
path.write_text(self.browser.page_source)
----
====
// CSANAD: I suggest placing a newline in front the print:
// print("\nscreenshotting to", path)
// or
// print("\n")
// print("screenshotting to", path)


And finally here's a way of generating a unique filename identifier,
Expand Down Expand Up @@ -900,7 +916,18 @@ dumping page HTML to ...goat-book/src/functional_tests/screendumps/MyListsTest.
test_logged_in_users_lists_are_saved_as_my_lists-2025-02-18T11.29.00.html
----

DAVID: Could get them to open those files and have a look? It's kind of satisfying!
// DAVID: Could get them to open those files and have a look? It's kind of satisfying!

// CSANAD: I just opened them, and they are blank, all-black images. Also, the
// HTML dump is just:
// `<html><head></head><body></body></html>`
// So maybe we could inform the Reader in case they do open these, that this is
// normal: due to the artificial self.fail() failure, this output is expected.

// CSANAD: however, running all the FT -s tells me something is probably wrong:
// `$ ./src/manage.py test functional_tests`
// creates a screenshot and an HTML dump for all FT -s.


=== Saving Build Outputs (or Debug Files) as Artifacts

Expand Down Expand Up @@ -937,7 +964,7 @@ test:


In any case that should work.
If you commit the code and then push it back to Gitlab,
If you commit the code and then push it back to GitLab,
we should be able to see a new build job.

[role="dofirst-ch25l010-1"]
Expand All @@ -953,6 +980,8 @@ $ *git push*
// so we can check it's working? (Partly it's because you're not so directive about where to put the fail so it
// feels more ephemeral.)

// CSANAD: the official book-example also removed the `self.fail()` by this point.
Copy link
Collaborator Author

@Xronophobe Xronophobe Apr 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an answer to David's comment


In its output, we'll see the screenshots and html dumps being saved:


Expand Down Expand Up @@ -984,6 +1013,21 @@ And if you navigate through, you'll see something like <<gitlab_ui_show_screensh
[[gitlab_ui_show_screenshot]]
image::images/gitlab_ui_show_screenshot.png["GitLab UI showing a normal-looking screenshot of the site"]

// CSANAD: working from the book-example, this is not true for me:
// https://gitlab.com/csberes/superlistz-version-harry/-/jobs/9765894394
// ```
// WARNING: src/functional_tests/screendumps/: no matching files. Ensure that the artifact path is relative to the working directory (/builds/csberes/superlistz-version-harry)
// ERROR: No files to upload
// Cleaning up project directory and file based variables 00:00
// Job succeeded
// ```
// The path looks OK, and locally, the test does create a `screenshots` directory.
// The directory does not get created in the Gitlab build job - I guess,
// because there are no errors. So, even though in my VM the official book-example
// version does generate a screenshot for every FT, it isn't true for the Gitlab.
// However, there is another odd thing: I tried re-adding the `self.fail`
// which resulted in three screenshots on Gitlab:
// https://gitlab.com/csberes/superlistz-version-harry/-/jobs/9766298077
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three screenshots, but only one failing test, which I made to fail.
Screenshot from 2025-04-19 00-33-56



=== If in Doubt, Try Bumping the Timeout!
Expand Down Expand Up @@ -1093,7 +1137,7 @@ Wrote configuration to spec/support/jasmine-browser.mjs.
----

Well we now have about a million files in _node_modules/_
(which is JavaScript's verions of a virtualenv essentially),
(which is JavaScript's version of a virtualenv essentially),
and we also have a new config file in _spec/support/jasmine-browser.mjs_.

That's not the ideal place, because we've said our tests live in a folder called _tests_,
Expand Down
Loading