=== _yogg is now known as codyshepherd
[15:25] <minimal> could do with some help with a testcase, the code it calls uses util.write_file but I can't mock.patch that as the testcase also needs to use it to create a file for the test
[15:25] <minimal> so I'm not sure how to achieve both - to "trap" the code doing its write_file and also to use write_file in the test
[15:48] <blackboxsw> minimal: can you util.write_file inline in the test before calling `m_write_file = mocker.patch("cloudinit.util.write_file")`  in the specific test?
[15:53] <blackboxsw> minimal: or you may want to just allow write_file to do what it wants and use the fake_filesystem python fixture which will retarget all util.write_file operations to a dir under /tmp
[15:56] <minimal> yeah I'm using helpers.FilesystemMockingTestCase to create a temp directory
[15:56] <minimal> let me post a gist of what I've got currently
[15:57] <blackboxsw> perfect, can help better then as we'd ❤️ to move away from FilesystemMockingTestCase as it's a pain, and fake_filesystem is the successor where possible
[16:00] <minimal> https://gist.github.com/dermotbradley/731f0d3bbfabeaa3aa824acb179d30f0
[16:01] <minimal> so function "test_busybox_add_user" is the testcase in question
[16:01] <minimal> on line 36 it creates a shadow file using util.write_file
[16:02] <minimal> on line 44 the create_user code will also call util.write_file (to modify shadow file)
[16:06] <minimal> so I just replace the testcase's util.write_file call with defining "content" to specify both the filename and the content and then call populate_dir accordingly?
[16:06] <blackboxsw> looking 
[16:29] <blackboxsw> minimal: I'd look at something like the following if possible https://dpaste.com/7LVC45BKR. Note that this test is still going to hit a runtime error as it mocks out all of subp with a return of False
[16:29] <blackboxsw> but it gets you far enough into the distro.lock_passwd method
[16:31] <blackboxsw> but it looks like that unit test is trying to validate the impact of actually calling passwd or usermod --lock maybe?
[16:32] <minimal> the testcase is for Busybox's adduser (rather than "useradd") but the code also "manually" modifies /etc/shadow as well as adduser does not provide the ability to set some /etc/shadow fields
[16:37] <blackboxsw> minimal: since this test case is mocking out all subp calls. nothing is actually invoking busybox right? So, we'll never get down to your actual validation of the /tmp/etc/shadow being changed will we?
[16:37] <blackboxsw> This test intent is just to validate that the calls that would be going to busybox have the correct params on the CLI.
[16:38] <blackboxsw> Is that true?
[16:39] <minimal> hmm, true....as adduser isn't actually run then nothing is written to /etc/shadow, but the code will still then try and update the (non-existent) entry in /etc/shadow....I guess I can just add that to the testcase's generated /etc/shadow contents
[16:40] <minimal> the actual code uses util.load_text_file and util.write_file to manipulate /etc/shadow
[16:40] <blackboxsw> for my context, this is relate  to https://github.com/canonical/cloud-init/pull/5176 I presume?
[16:40] -ubottu:#cloud-init- Pull 5176 in canonical/cloud-init "feat(alpine): add support for Busybox adduser/addgroup" [Open]
[16:40] <minimal> yupe
[16:41] <minimal> I'm just finishing up with some additional changes and this is the last testcase to sort out
[16:42] <blackboxsw> ok roger. switching to your PR so I can get bearings on the test need
[16:51] <blackboxsw> minimal: https://dpaste.com/AY6H5XMYR maybe? validate the commands you think should be called
[16:51] <blackboxsw> the above passes on your branch
[16:52] <blackboxsw> and additionally you'd want to load that shadow_file afterward to ensure the write_files worked... I think you your PR you'll maybe want to use util.load_text_file instead of a direct call to open(), so we can log what cloud-init reads from the filesystem
[16:53] <blackboxsw> I'll go through that PR now and add a couple of review comments
[16:53] <minimal> yeah I've already changed the code to use util.load_text_file
[16:54] <minimal> there's no point reviewing it yet until I push my additional code changes
[17:02] <blackboxsw> sounds good, note that the restructure of the unittest in question doesn't setup expirydate or inactive as not-None, so /etc/shadow isn't altered in the tmpdir
[17:02] <blackboxsw> will let you work out additional code changes before peeking again
[17:02] <minimal> blackboxsw: /etc/shadow is altered because of some additional changes I've made locally ;-)
[17:03] <blackboxsw> I've misplaced my ssh keys to your home system and bank accounts etc. So, I'll just await a ping or notification on the PR :
[17:03] <blackboxsw> :)
[17:17] <minimal> blackboxsw: thanks for your help, making progress, now having testcase fail for another reason (going to have to handle mocking of subp calls for "addpasswd" and "passwd" separately)
[17:18] <minimal> as code's subp call of "passwd --help" expects stdout/stderr output