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

Update modulesync and fix test #4

Closed
wants to merge 3 commits into from

Conversation

Valantin
Copy link

@Valantin Valantin commented Apr 10, 2024

Pull Request (PR) description

update modulesync

This Pull Request (PR) fixes the following issues

Include #5 #6

@Valantin Valantin force-pushed the fix-test branch 3 times, most recently from efd78b4 to f73954a Compare April 10, 2024 12:08
it 'contains the default fstab setting' do
shell('cat /etc/fstab | grep /tmp/swapfile.fallocate', acceptable_exit_codes: [0])
end
describe file('/etc/fstab'), shell('/sbin/swapon -s') do
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you can combine the shell part in here.

Actually, our code doesn't use this a lot but it's recommended to avoid describe $resource() and use:

it 'sets up fstab' do
  expect(file('/etc/fstab')).to be_file
    .and(have_attributes(content: include('/mnt/swap.1')))
end

Copy link
Author

@Valantin Valantin Apr 10, 2024

Choose a reason for hiding this comment

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

and if I want to check the output of a shell script?

I've tried your example and it fails

-:content => (include "/mnt/swap.1"),
 +:content => "# /etc/fstab: static file system information.\n#\n# <file sys>\t<mount point>\t<type>\t<options>\t<dump>\t<pass>\n\n# device during installation: /dev/loop0p1\nUUID=e40706ce-053f-451f-b6fe-0979430677a4\t/\text4\trw,discard,errors=remount-ro\t0\t1\n#VAGRANT-BEGIN\n# The contents below are automatically generated by Vagrant. Do not modify.\n#VAGRANT-END\n",

@sahaqaa
Copy link

sahaqaa commented Sep 18, 2024

Hello, please advice, is there any chance to get this PR merged? Thank you.

Looking forward to seeing first release of this module to be released to Forge.

@rwaffen
Copy link
Member

rwaffen commented Sep 20, 2024

is this one still needed? if so, please rebase, you seem to have some conflicts now 🙈

@traylenator
Copy link

Not needed anymore - thanks.

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.

6 participants