Skip to content
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.

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

nixosTests.sourcehut: implement proper integration testing #271324

Merged
merged 13 commits into from
Feb 17, 2024

Conversation

nessdoor
Copy link
Contributor

@nessdoor nessdoor commented Dec 1, 2023

Background

During the review process of #245394, it turned out that large parts of the Sourcehut module have been left to rot due to the (apparently) small size of both its user base and the subset of functionalities that most users actually deploy, in addition to the relatively unstable nature of the Sourcehut software, which can take a considerable amount of maintenance effort in order to be kept on track with upstream changes.

Excluding a sudden rise in popularity of NixOS-deployed Sourcehut instances, this slow drift into functional decay can only be combated through the effective use of automated integration testing, which can quickly point maintainers at breakages following version bumps in either the Sourcehut package set or of the modules the Sourcehut module depends on.

To make matters worse, the Sourcehut module is very complex to understand and refactor, due to the considerable amount of changes introduced over time and the distributed nature of the services it manages. This, in turn, leads to a very challenging review process, which can stall critical PRs for a very long time and quickly exhaust the energy of reviewers.

Proper integration testing is critical to lower this maintenance and reviewing burden, as it enables quick feedback even to large multi-module refactoring efforts and gives reviewers some confidence that the changes being introduced do not break any existing functionality.

Description of changes

The current integration tests targeting the Sourcehut module are very limited, as they only verify that services come up at boot and that web servers responds to GET requests. Not only that, they also just verify three services (meta, git and part of builds), while the list of Sourcehut services is far longer than that.

This PR introduces the basis on which to build better integration tests for the entire module, based on both scripted interaction with the Web front-end and direct querying of the service APIs, as anticipated in #267538. The current objective is to fully test the interaction of the two core components that most people seem to be using: meta and git. Nonetheless, we also aim at laying the infrastructure necessary to extend the battery of tests, as more sub-modules get updated and refactored over time. The final goal is that of achieving very high inter-service integration testing coverage, complete with webhooks and other notification mechanisms, through a series of subsequent contributions.

Draft

This PR will begin as a draft, as all the necessary work has not been completed, yet. The decision to "go public" before completion is because this is a somewhat large undertaking, and it is, therefore, very important to de-duplicate effort, signal to the community that things are moving and, hopefully, attract some helping hands along the way.

The draft status will be lifted as soon as the stated objective (meta and git integrated testing) will be reached within acceptable margins.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Priorities

Add a 👍 reaction to pull requests you find important.

At the time of this commit, sourcehut.builds is broken and its test
infrastructure is partial at best. The specified image only inflated the closure
size without any added benefit.
@nessdoor nessdoor marked this pull request as ready for review December 25, 2023 14:49
@nessdoor
Copy link
Contributor Author

Undrafting this now.

The core integrated functionality of a Git code forge has been tested, and going any deeper than this starts to uncover some non-fatal issues that must be addressed separately (e.g. API errors when querying the GraphQL endpoint directly). Additional tests for the rest of the services will be added once their correct base integration has been verified.

Additionally, the outcome of these tests seem to indicate that #138511 is no longer an issue.

Fixes a bug where, if the OpenSSH server starts before the first initialization
of the git service, the SSH service fails to find the corresponding bind mount
and terminates with a 226/NAMESPACE error.
Copy link
Contributor

@christoph-heiss christoph-heiss left a comment

Choose a reason for hiding this comment

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

Whole testsuite runs through w/o issues now. 👍
Diff also LGTM.

Thanks for all the work!

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1403

Promote the srht-gen-oauth-tok package to a Nixpkgs package.
@nessdoor
Copy link
Contributor Author

nessdoor commented Feb 7, 2024

@SuperSandro2000 I have moved the package as you suggested, thank you for your comments!
Please, let me know if you have anything else you want to see me change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants