kata-irc-bot | <bradenr> Hi, I've got a question regarding some of the runtime unit tests, specifically the kata-check and kata-env ones that are checking certain CPU information. One of the tests in question is in the `kata-check_amd64_test.go` file, the `TestSetCPUtype` test. The test is expecting `archRequiredCPUFlags["vmx"]` to be set. As far as I can tell, this flag is only set on Intel CPUs, and the CPU information it is checking does not seem to be | 01:48 |
---|---|---|
kata-irc-bot | mocked. I am currently on an AMD cpu, so the flag is not being set, and the test is failing. Anyone have any insight into this? Thanks! | 01:48 |
kata-irc-bot | <ssheribe> @eric.ernst could you refer to the changes on our side? | 08:30 |
kata-irc-bot | <james.o.hunt> Hi @bradenr - AMD support was added by AMD and Oracle but it looks like it may not have fully extended into the test code ;) fwics `assert.Equal(archRequiredCPUFlags["vmx"], "Virtualization support") ` should indeed have a check on the cpu type by calling something like `cpuType = getCPUtype()` and then checking `cpuType` against `cpuTypeIntel` or `cpuTypeAMD` (you could add in a check to ensure `archRequiredCPUFlags` contains | 09:30 |
kata-irc-bot | `cpuFlagSVM` on that system). | 09:31 |
kata-irc-bot | <jakob.naucke> We are currently also dismissing reviews whenever there is an update to the base branch. Maybe I had missed that yesterday, but were we aware of this? | 11:12 |
kata-irc-bot | <fidencio> I don't think we were aware of this, no. | 11:48 |
kata-irc-bot | <fidencio> That's worth a bug on GitHub itself? | 11:51 |
kata-irc-bot | <fidencio> What I did was: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repo[…]rgeability-of-pull-requests/managing-a-branch-protection-rule | 11:51 |
kata-irc-bot | <fidencio> """ """ | 11:51 |
kata-irc-bot | <fidencio> By the way, could someone, please, approve https://github.com/kata-containers/kata-containers/pull/3504 ? | 11:53 |
kata-irc-bot | <jakob.naucke> https://github.com/kata-containers/tests/pull/4376#pullrequestreview-862616131 I think all that happened here is that the new policy went into effect, which is why David's and Wainer's approvals are dismissed, but not James's. | 12:01 |
kata-irc-bot | <fidencio> Okay, so we keep monitoring for weirdness, but we just did a quick test on the `kata-containers` repo and that didn't happen. | 12:03 |
kata-irc-bot | <jakob.naucke> @fidencio wait, did you or @gabriela.cervantes.te set that in the tests repo? Just trying to understand why the dismissal comes from Gaby and I had thought it was the update to the base branch because there was a merge from Gaby around a similar time. | 12:03 |
kata-irc-bot | <fidencio> Checking the differences between the `kata-containers` and `tests` repo, there's not much, apart from """ Require review from Code Owners Require an approved review in pull requests including files with a designated code owner. """ on the tests repo. | 12:04 |
kata-irc-bot | <fidencio> I was the one who set that on all repos, Yesterday after the A/C meeting. | 12:04 |
kata-irc-bot | <jakob.naucke> Huh. | 12:05 |
kata-irc-bot | <fidencio> The first person to commend on a PR where that policy takes effect will be the one marked as "dismissing the review" | 12:46 |
kata-irc-bot | <jakob.naucke> Funky. | 12:46 |
kata-irc-bot | <fidencio> But not really, as Gaby didn't comment there :( | 12:47 |
kata-irc-bot | <fidencio> It happened on a few where I did comment on something | 12:47 |
kata-irc-bot | <jakob.naucke> Might have something to do with who merged another PR. | 12:47 |
kata-irc-bot | <bradenr> Thanks for confirming, I was beginning to suspect that those tests just needed updating. | 14:14 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!