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

test-driver.py: defaulting keepVmState in Machine init #97746

Merged
merged 1 commit into from Sep 11, 2020

Conversation

picnoir
Copy link
Member

@picnoir picnoir commented Sep 11, 2020

Motivation for this change

ecb73fd introduced a new keepVmState
CLI flag for test-driver.py. This CLI flags gets forwarded to the
Machine class through create_machine.

It created a regression for the boot tests where main end up not
being evaluated. See
#97346 (comment) for
bug report.

Things done

Defaulting keepVmState to false when main ends up not being
evaluated.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg test boot

ecb73fd introduced a new keepVmState
CLI flag for test-driver.py. This CLI flags gets forwarded to the
Machine class through create_machine.

It created a regression for the boot tests where __main__ end up not
being evaluated. See
NixOS#97346 (comment) for
bug report.

Defaulting keepVmState to false when __main__ ends up not being
evaluated.
Copy link
Member

@vcunat vcunat left a comment

Choose a reason for hiding this comment

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

Seems good at a quick look (I don't know this code). The four tests that regressed in -small get fixed by this.

@vcunat vcunat merged commit 2bb1868 into NixOS:master Sep 11, 2020
@flokli
Copy link
Contributor

flokli commented Sep 11, 2020

This also needs to be backported to 20.09, right?

@picnoir
Copy link
Member Author

picnoir commented Sep 11, 2020 via email

@picnoir picnoir deleted the nin-fix-boot-tests branch September 11, 2020 11:50
@vcunat
Copy link
Member

vcunat commented Sep 11, 2020

I have it... just retrying some tests locally before pushing, to be sure.

vcunat added a commit that referenced this pull request Sep 11, 2020
(cherry picked from commit 2bb1868)
I re-checked some of the regressed tests on 20.09.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants