Skip to content

Fix for #420 (Conditionals must have a boolean result.) and basic tests for #425#426

Merged
guenhter merged 5 commits intoriemers:masterfrom
joannakotula-enghouse:master
Feb 5, 2026
Merged

Fix for #420 (Conditionals must have a boolean result.) and basic tests for #425#426
guenhter merged 5 commits intoriemers:masterfrom
joannakotula-enghouse:master

Conversation

@joannakotula-enghouse
Copy link

@joannakotula-enghouse joannakotula-enghouse commented Feb 4, 2026

Fix is in the first commit - simple one line

Tests:

I've created basic tests with only one mode tested (by_template) on Ubuntu 22.04, in both greenfield and brownfield.
I leave it that way to make it easier for someone else to close (eventually) #425 - I don't have enough time.

To run the test, install requirements from molecule/requirements.txt and run the run_tests.sh script.

I'm not adding anything to README because:

  • information about tests to run before submitting PR should be in the CONTRIBUTING.md file (I've read a suggestion that it should be .github/CONTRIBUTING.md file ;) )
  • I believe that the maintainer should create the file 😉
  • I assume that someone will take the tests I created, add some cases, and include them in GitHub Actions, and then decide what about the README/CONTRIBUTING notes

@guenhter
Copy link
Collaborator

guenhter commented Feb 4, 2026

Could you have a look at the linting errors?

@joannakotula-enghouse joannakotula-enghouse force-pushed the master branch 3 times, most recently from 628d668 to 612f2cf Compare February 4, 2026 14:33
Joanna Kotuła added 2 commits February 4, 2026 15:36
removed old tests, used only mock server
removed pidfile as not necessary anymore
@joannakotula-enghouse
Copy link
Author

@guenhter lint test passed 😃

---
galaxy_info:
author: Erik-jan Riemers
role_name: gitlab_runner
Copy link
Collaborator

Choose a reason for hiding this comment

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

@riemers Changes in this file always ring alarm bells. Do you think this is again a problem with ansible galaxy? I mean the change should be done as the linter complained...

Choose a reason for hiding this comment

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

It was needed for the molecule to use the role in the converge.yml playbook. It complained that the name wasn't set, as I remember.

I'm sorry, I missed that I used an "underscore" instead of a "dash" in the role name 🙈 I can revert it, remove the role_name check from the lint, and, as a role to test, use "{{ playbook_dir }}/../..". What do you think?

Choose a reason for hiding this comment

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

Import by path works in converge.yml, but molecule still doesn't like the role name. I'm not sure how to fix it :(

@riemers
Copy link
Owner

riemers commented Feb 4, 2026 via email

@guenhter
Copy link
Collaborator

guenhter commented Feb 5, 2026

@riemers do you think I can merge that change?

@riemers
Copy link
Owner

riemers commented Feb 5, 2026 via email

@guenhter guenhter merged commit 7bc6613 into riemers:master Feb 5, 2026
1 check passed
@guenhter
Copy link
Collaborator

guenhter commented Feb 5, 2026

Nice. @joannakotula-enghouse thx for the effort

@riemers
Copy link
Owner

riemers commented Feb 5, 2026

It created riemers.gitlab_runner in ansible galaxy. We can always tell folks to move to that one. I have now 3 different versions, but i just update them all when i release something new anyway. 1 i have to remove yet, but lets worry on that later ;p

@riemers
Copy link
Owner

riemers commented Feb 5, 2026

also @guenhter now we need to do something with the contribution md files and github actions etc etc.. time is a problem we all have it seems ;P

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

Comments