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

Add test for chocolate-doom #20661

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

okurz
Copy link
Member

@okurz okurz commented Nov 21, 2024

doom was mentioned during a common criteria auditing meeting at SUSE as a fun side-note but I am also taking those seriously.

Verification run: https://openqa.opensuse.org/tests/4658592

doom was mentioned during a common criteria auditing meeting at SUSE as
a fun side-note but I am also taking those seriously.
Copy link

github-actions bot commented Nov 21, 2024

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

  • Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files.

@okurz okurz marked this pull request as ready for review November 21, 2024 10:24
Copy link
Member

@foursixnine foursixnine left a comment

Choose a reason for hiding this comment

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

This is only testing the chocolate-doom launcher - doesn't actually run/test doom and you would need to get written permission to add a wad file (unless you make your own)

I am also taking those seriously.

Perhaps too seriously :)

Also leave it only on development jobgroups.

@okurz
Copy link
Member Author

okurz commented Nov 21, 2024

This is only testing the chocolate-doom launcher - doesn't actually test chocolate-doom and you would need to get written permission to add a wad file (unless you make your own)

Yes, this is only testing the chocolate-doom launcher which is the essential part of the package. I don't see that as a hindrance.

Also leave it only on development jobgroups.

Why should we? I verified the test and don't see a need to stabilize further in a development job group.

@foursixnine
Copy link
Member

This is only testing the chocolate-doom launcher - doesn't actually test chocolate-doom and you would need to get written permission to add a wad file (unless you make your own)

Yes, this is only testing the chocolate-doom launcher which is the essential part of the package. I don't see that as a hindrance.

It isn't a hinderance, the test is incomplete. The error message you get, is way before anything besides default configuration is loaded: https://github.com/chocolate-doom/chocolate-doom/blob/bb9bb2324852db731d81ee8c1f16a9b0b0fc31df/src/doom/d_main.c#L1496

Also leave it only on development jobgroups.

Why should we? I verified the test and don't see a need to stabilize further in a development job group.

See above. The test isn't necessarily unstable, but is not testing any functionality.

Copy link
Member

@asmorodskyi asmorodskyi left a comment

Choose a reason for hiding this comment

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

LGTM

@asmorodskyi
Copy link
Member

See above. The test isn't necessarily unstable, but is not testing any functionality.

it does :

  1. package installation
  2. even to get to this first window you can get some exception if installation totally broke

ofc. it is very minimal set , but correct me if I am wrong it is more then we have in master today , right ?

@asmorodskyi
Copy link
Member

P.S. in case you wonder what I am doing in this PR , it is very simple I got caught by PR name :D

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

Successfully merging this pull request may close these issues.

3 participants