Clear environment variable efi_selftest before executing the
default tests.
Provide a test verifying that the EFI watchdog
reboots the system upon timeout.
The test depends on CONFIG_CMD_EFI_SELFTEST=y.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Alexander Graf <agraf@suse.de>
It can be useful to record how long tests take; this can help debug slow
running test systems or track changes in performance over time. Enhance
the test system to record timestamps while running test:
- Whenever a new log file section is started.
- After U-Boot is started and communication has been established.
- After each host or U-Boot command is executed.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
test_gpt generates a persistent disk image which can be re-used across
multiple test runs. Currently, if the Python code that generates the disk
image change, the image is not regenerated, which could cause test
failures e.g. if a test was updated to expect some new partition name or
size, yet the persistent disk image contained the old name or size. This
change introduces functionality to regenerate the disk image if the
instructions to generate the image have changed.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
don't use prettyprint_part_size() in create_gpt_partitions_list()
that avoid to align offset and size to 1 MiB and increase precision for
start and size.
This patch avoid the risk to change partition size and lost data during
rename or swap.
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
Add test of first and last LBA in gpt for rename and swap.
Only the name is expected to change, so test 3 columns
for part command
1: first LBA (start)
2: last LBA (end)
3: partition name
After rename, the last LBA change and it is a error in current U-Boot code
+ "first" = 0x7ff : invalid value (<start)
+ "second" = 0x17ff => size increasing !
Acked-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
+ test write for one partition on all the device (size=0)
+ test write with disk uuid and 2 partitions
Acked-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
add sandbox test for some gpt sub-command
- gpt read / part list : read the gpt partition created by sgdisk on host
test start, size, LBA and name output
- gpt verify : verify the gpt partition create by sgdisk on host
PS: persistent data test_gpt_disk_image.bin are udpated
Acked-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
copy the persistent gpt binary file as it can be modified during the test
that avoid issue if the test fail: the test always restart with clean file
Acked-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
On qemu errors like
assert 2.999650001525879 >= 3
occur.
According to the comment in the code the test is meant to be
approximate. So we should accept some milliseconds less.
Cc: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
Lots of new things this time. High level highlights are:
- Shim support (to boot Fedora)
- Initial set of unit tests
- Preparations to support UEFI Shell
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAABAgAGBQJZ0KdpAAoJECszeR4D/txga+AQAL8ehBlkr3R6b/n00p3kuAVE
cfM+o/16b+P/r3U2/wvlNtw7L+UPlGaQNk8o7n4xTu0DpVlIt7DhFrSBIcK6VbYB
8BFJ/EL9NRj/sd/wb9B1Bc0SlXJcGxzRM+JxNseVmc7rz9sWdARvx/diQlP+YAX4
qjp2YRzsErjUgix4eGDXvgFQdFmh4Pnp7UnHlSq4WkINo14navGLe/PwChxsL91M
AZV7JulVcm7XtOPTON8udec3O15O8BExAM1cH2ITjtD99Je+4VcuI99nLm2aIE++
U6vTuoogHAU+oCutD3qZU85uj2VJ2kYy2k5A/RK/i6Ug/QrTa+JA8PNWP78/TZ+8
xmyDVzG5MQhMIddDjJlMXbxIwgKhr0SjUJ1iVaGI0bS/6Glc7h9m+hhIXKudqOvs
lxqPhy7mahtBwETjoUlM1WJg+TG+vIGDJtT22njjOSTh0CVYw2H3xihyFKE3AjaW
L26EOEsxArErtzF17GgF5KxFOGaYAmVmA3XnCcEZEIY5gzL65uok1GX/apf4sIsi
j8HSbW3UH6QwjWKBhF+RVuK4CcTDUS9K2YmJlrdZ69elDZyg5PWbpMLNdYIP+VVG
+j91c7e3O2q2AHAzTprpmkyeIeqZfFhgyu4H/jPQ8Yi8N/b7wbugcLm3fHuUNal4
RB0KjbyJzJPQwbYGSLYi
=9HkH
-----END PGP SIGNATURE-----
Merge tag 'signed-efi-next' of git://github.com/agraf/u-boot
Patch queue for efi - 2017-10-01
Lots of new things this time. High level highlights are:
- Shim support (to boot Fedora)
- Initial set of unit tests
- Preparations to support UEFI Shell
The current code wraps a pre tag inside an a tag. For some reason, this
causes at least Firefox to attempt to drag the pre section content when
using a mouse drag to select text. Re-order the tags so that the text can
be selected using the mouse, at least if you start the drag outside the
text (after the end of the line, for example).
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Some tests use external tools (executables) during their operation. Add
a test.py mark to indicate this. This allows those tests to be skipped if
the required tool is not present.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
When skipping tests, explicitly mention the board type or config option
that caused the skip. This will help people understand/fix any issues.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
A Python test script is provided that runs the EFI selftest
if CONFIG_CMD_EFI_SELFTEST=y.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Alexander Graf <agraf@suse.de>
Make various changes to the GPT test:
1) Reference the disk image using an absolute path in all cases. This
allows test/py to operate correctly if it's run from a directory other
than the root of the U-Boot source tree.
2) Store the disk image in the teswt/py persistent data directory. This
removes the need to re-generate it every time the tests are run.
3) Execute sgdisk using u_boot_utils.run_and_log() so that its output is
captured in the test log. This allows debugging any problems running it.
4) Make the disk image a test fixture. This removes the requirement to
always run all GPT tests, and run them in order. The current code doesn't
create the disk image if e.g. just test_gpt_uuid() is executed via the
test.py -k command-line option.
5) Use @pytest.mark.buildconfigspec for all feature dependencies, rather
than manually implementing some of them.
6) Make all tests depend on sandbox, since they use the sandbox-specific
host command.
Fixes: a2f422555f ("add pytests for 'gpt guid' command in sandbox")
Fixes: c5772188ed ("add pytests for 'gpt rename' and 'gpt swap'")
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Tom Rini <trini@konsulko.com>
Add unit tests for the 'gpt rename' and 'gpt swap' commands that
rely on the block device created by test/py/make_test_disk.py.
Add CONFIG_CMD_GPT_RENAME to the sandbox_defconfig. Remove the
testdisk.raw test device at the end of the tests.
Signed-off-by: Alison Chaiken <alison@peloton-tech.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Run unit tests for the 'gpt guid' command, making use of the block
device created by test/py/make_test_disk.py. Remove this device at
the end of the tests.
Signed-off-by: Alison Chaiken <alison@peloton-tech.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Provide a Python function that creates a small block device for the
purpose of testing the cmd/gpt.c or cmd/part.c functions in the u-boot
sandbox.
Signed-off-by: Alison Chaiken <alison@peloton-tech.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
If one does not already have a rule to create a custom device node when
a given device enumerates it can be useful to have udev create a
bus path based node to the entry in /dev/bus/usb that was just
enumerated. Given that DFU itself does not require a /dev entry it is a
good idea to provide a rule that will generate one.
Signed-off-by: Tom Rini <trini@konsulko.com>
Currently we have code which prints out platform data at the start of SPL.
Now that we have tests for dtoc this is probably not necessary. Drop it.
Update test_ofplatdata to check for empty output since it is useful to
check that sandbox_spl works as expected.
Signed-off-by: Simon Glass <sjg@chromium.org>
Some tests depends on echo command to be present.
Reported-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
All tests in test_hush_if_test depends on hush parser to be
present. This patch simplify test dependencies by using global
pytestmark.
Reported-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
There is missing dependency on echo command. Mark tests which requires
echo.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
We have all the building blocks now to run arbitrary efi applications
in travis. The most important one out there is grub2, so let's add
a simple test to verify that grub2 still comes up.
Signed-off-by: Alexander Graf <agraf@suse.de>
Today we can compile a self-contained hello world efi test binary that
allows us to quickly verify whether the EFI loader framwork works.
We can use that binary outside of the self-contained test case though,
by providing it to a to-be-tested system via tftp.
This patch separates compilation of the helloworld.efi file from
including it in the u-boot binary for "bootefi hello". It also modifies
the efi_loader test case to enable travis to pick up the compiled file.
Because we're now no longer bloating the resulting u-boot binary, we
can enable compilation always, giving us good travis test coverage.
Signed-off-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Tom Rini <trini@konsulko.com>
Now that we have working network tests and a hello world efi application
built inside our tree, we can automatically test that efi binary running
inside of U-Boot.
Signed-off-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
The network test currently downloads files at 0MB offset of RAM start.
This works for most ARM systems, but x86 has weird memory layout constraints
on the first MB of RAM.
To not get caught into any of these, let's add a 4MB pad from start
of RAM to the default memory offset.
Signed-off-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
If a test uses a fixture which is expensive to setup, the fixture can
possibly created with session or module scope. As u_boot_console has
function scope, it can not be used in this case.
Signed-off-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
Acked-by: Stephen Warren <swarren@wwwdotorg.org>
In pytest 3, runtestprotocol() may not call pytest_runtest_setup() if
the test is skipped. That call is required to create a section for the
test in the log file. If this is skipped, the call to log.end_section()
at the tail of pytest_runtest_protocol() will throw an exception. This
patch ensures that a log section always exists, both to avoid the
exception and to ensure that a consistently structured log file is
always created.
Cc: Stefan Brüns <stefan.bruens@rwth-aachen.de>
Reported-by: Stefan Brüns <stefan.bruens@rwth-aachen.de>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Tom Rini <trini@konsulko.com>
Add a NFS download test, based on TFTP test.
Tested on i.MX6 SabreLite board.
Signed-off-by: Guillaume GARDET <guillaume.gardet@free.fr>
Cc: Tom Rini <trini@konsulko.com>
Cc: Joe Hershberger <joe.hershberger@ni.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Simon Glass <sjg@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Joe Hershberger <joe.hershberger@ni.com>
Make sure that when we're telling bootm to boot an image, and we expect
the image to boot we get the output from sandbox that we attempted to
run Linux and that U-Boot completed its job.
Cc: Simon Glass <sjg@chromium.org>
Cc: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Tom Rini <trini@konsulko.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Acked-by: Stephen Warren <swarren@nvidia.com>
This prevents capture of command output from terminating early on boards
that use a simple prompt (e.g. "=> ") that appears in the middle of
command output (e.g. crc32's "... ==> 2fa737e0").
Reported-by: Tom Rini <trini@konsulko.com>
Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
Tested-by: Tom Rini <trini@konsulko.com>
Reviewed-by: Heiko Schocher <hs@denx.de>
Return one string for each command that was executed. This seems cleaner.
Suggested-by: Teddy Reed <teddy.reed@gmail.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
Add a proper function for this rather than using internal functions. Use it
in the single call site.
Also, do a restart at the end of the vboot test to reset to the normal
device tree.
Signed-off-by: Simon Glass <sjg@chromium.org>
Suggested-by: Stephen Warren <swarren@nvidia.com>
Use 'cons.log.section' feature to split up the test output. This makes it
easier to read.
Suggested-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
Rename this argument and pass it to each function that needs it, instead of
making it global.
Suggested-by: Stephen Warren <swarren@nvidia.com>
Suggested-by: Teddy Reed <teddy.reed@gmail.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
Instead of this, use the existing run_and_log() function, enhanced to
support a command string as well as a list of arguments.
Suggested-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
Fix some typos in various files introduced with the vboot test conversion.
Reported-by: Teddy Reed <teddy.reed@gmail.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
This check was missed. Add it and make the message more verbose.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reported-by: Tom Rini <trini@konsulko.com>
Fixes: 9e17b034 (test/py: Provide a way to check that a command fails)
Fix review comments that were missed at the time. Also explain why we need
to regenerate the device tree for each test.
Reported-by: Teddy Reed <teddy.reed@gmail.com>
Suggested-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
Fixes: f6349c3c (test: Add a README)
Getting this error:
Zynq> sb load hostfs - 100
/home/monstr/data/disk/u-boot/build-zynq_zc706/test.fit
Unknown command 'sb' - try 'help'
because sb command is present only for Sandbox
obj-$(CONFIG_SANDBOX) += host.o
that's why mark this test to be run only at Sandbox
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Simon Glass <sjg@chromium.org>
Without this, the test fails if the test is run with a cwd other than the
root of the U-Boot source tree.
Fixes: 8729d58259 ("test: Convert the vboot test to test/py")
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
This should be spl_of_platdata, since otherwise it will try to run on boards
that don't support of-platdata.
Signed-off-by: Simon Glass <sjg@chromium.org>
Add a simple test which checks that the of-platdata system is working
correctly. The sequence is as follows:
- SPL starts up and probes all the UCLASS_MISC drivers
- There are 3 of these in sandbox.dts
- Therefore there should be 3 U_BOOT_DEVICE() declarations in dt-platdata.c
- These should produce 3 sandbox_spl_test devices
- Each device prints out its platform data when probed
- This test checks for this output and compares it against expectations
Signed-off-by: Simon Glass <sjg@chromium.org>
When sandbox SPL is enabled we want to start that rather than U-Boot proper,
since some tests may rely on running it first.
Signed-off-by: Simon Glass <sjg@chromium.org>
At present the SPL and U-Boot consoles both present the same error message
when the expected console output does not appear. Add "SPL" to the SPL error
message to resolve this ambiguity.
Signed-off-by: Simon Glass <sjg@chromium.org>
This board can sometimes be used for tests. Handle it the same way as
sandbox.
Note: I plan to drop the sandbox_spl board at some point and merge its
features into sandbox. So this commit may not be necessary.
Signed-off-by: Simon Glass <sjg@chromium.org>
Now that we have a suitable test framework we should move all tests into it.
The vboot test is a suitable candidate. Rewrite it in Python and move the
data files into an appropriate directory.
Signed-off-by: Simon Glass <sjg@chromium.org>
Some tests want to execute a sequence of commands. Add a helper for this.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Teddy Reed <teddy.reed@gmail.com>
Sometimes we want to run a command and check that it fails. Add a function
to handle this. It can check the return code and also make sure that the
output contains a given error message.
Signed-off-by: Simon Glass <sjg@chromium.org>
It is sometimes inconvenient to convert a string into a list for execution
with run_and_log(). Provide a helper function to do this.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Teddy Reed <teddy.reed@gmail.com>
It is useful to be able to obtain the output from a command. Return it from
this function.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Teddy Reed <teddy.reed@gmail.com>
Tests may want to look at the output from running a command, even if it
fails (e.g. with a non-zero return code). Provide a means to obtain this.
Another approach would be to return a class object containing both the
output and the exception, but I'm not sure if that would result in a lot
of refactoring.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Teddy Reed <teddy.reed@gmail.com>
Tests may want to look at the output from running a command. Return it so
that this is possible.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Teddy Reed <teddy.reed@gmail.com>
Normally tests will run with the test.dtb file designed for this purpose.
However, the verified boot tests need to run with their own device-tree
file, containing a public key.
Make the device-tree file a config option so that it can be adjusted by
tests. The default is to keep the current behaviour.
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Teddy Reed <teddy.reed@gmail.com>
Some archs like MIPS or PPC have a different 'bdinfo' output
than ARM regarding the memory configuration. Also support
'memstart = 0x*' in u_boot_utils.find_ram_base() to make
all tests requiring the RAM base working on those archs.
Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
Prior to this patch, any VT100 codes emitted by U-Boot are considered part
of a command's output, which often causes tests to fail. For example,
test_env_echo_exists executes printenv, and then considers any text on a
line before an = sign as a valid U-Boot environment variable name. This
includes any VT100 codes emitted. When the test later attempts to use that
variable, the name would be invalid since it includes the VT100 codes.
Solve this by stripping VT100 codes from the match buffer, so they are
never seen by higher level test code.
The codes are still logged unmodified, so that users can expect U-Boot's
exact output without interference. This does clutter the log file a bit.
However, it allows users to see exactly what U-Boot emitted rather than a
modified version, which hopefully is better for debugging. It's also much
simpler to implement, since logging happens as soon as text is received,
and so stripping the VT100 codes from the log would require handling
reception and stripping of partial VT100 codes.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Some CONFIG_ variables were recently renamed, but test/py wasn't updated
to match. This causes some tests to be skipped. Fix test/py so the tests
are run.
Fixes: 1163625898 ("Rename reset to sysreset")
Fixes: f1f9d4fac5 ("hush: complete renaming CONFIG_SYS_HUSH_PARSER to CONFIG_HUSH_PARSER")
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
CONFIG_VERSION_VARIABLE isn't always defined, so we can't simply look up
its value directly, or an exception will occur if it isn't defined.
Instead, we must use .get() to supply a default value if the variable
isn't defined.
Fixes: da37f006e7 ("tests: py: disable main_signon check for printenv cmd")
Acked-by: Heiko Schocher <hs@denx.de>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
if CONFIG_VERSION_VARIABLE is set, the U-Boot environment
contains a "vers" variable with the current U-Boot version
string. If now "printenv" is called, test/py fails as it
detects the main_sign string, which is in this case correct.
So check only the main_sign as an error, if CONFIG_VERSION_VARIABLE
is not set.
Signed-off-by: Heiko Schocher <hs@denx.de>
Large file transfers, flash erasing and more complicated tests
requires more time to finish. Provide a way to setup specific
timeout directly in test.
For example description for 50s test:
timeout = 50000
with u_boot_console.temporary_timeout(timeout):
u_boot_console.run_command(...)
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
test/py raises an error, if a board has not enabled bdi command
> pytest.skip('bdinfo command not supported')
E NameError: global name 'pytest' is not defined
import pytest in test/py/u_boot_utils.py fixes this.
Signed-off-by: Heiko Schocher <hs@denx.de>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
test/py raises an error, if a board has not enabled bdi command
> pytest.skip('bdinfo command not supported')
E NameError: global name 'pytest' is not defined
import pytest in test/py/u_boot_utils.py fixes this.
Signed-off-by: Heiko Schocher <hs@denx.de>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
It can take a while for a host machine to notice that a USB device has
disconnected, and process the change. At the end of the DFU test, we wait
up to 10 seconds for this to happen. This change makes the test wait the
same (up to) 10 seconds at the start of the test for any previously active
USB device-mode session to be cleaned up. Such as session might have been
used to download U-Boot into memory for example; this is certainly true
on my Tegra test systems. This changes should solve the DFU test
intermittency issues I've been seeing on some Tegra devices.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
After concatenation of "dfu_alt_info" variable from "dfu_alt_boot" and
"dfu_alt_system" it may happen that test and dummy files alt settings
are different than default 0 and 1.
This patch provides the ability to set different values for them.
Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
---
Changes for v3:
- replace variables declarations with ones read from configuration file
- remove not necessary str() conversion at DFU host command generation
Changes for v2:
- generate "alt_info" automatically
- use file names as alt settings instead of numerical values
- extend in-code documentation
By default (on almost all systems) the dfu env variable, which defines
available alt settings, is named as "dfu_alt_info".
However on some platforms (i.e. Odroid XU3), the 'dfu_alt_info' is concatenated
from other variables - namely 'dfu_alt_boot' and 'dfu_alt_system' at run time
(when one types 'dfu 0 mmc 0' for first time).
'dfu_alt_boot' describes alt settings which depend on boot medium - for example
boot loader's LBA sectors which are different on eMMC and SD card because of e.g.
MBR/GPT.
'dfu_alt_system' describes board agnostic alt settings - like rootfs, kernel.
On such system we can only append/modify this env variable.
Because of the above, we must have way to modify other than "dfu_ale_info"
variable to perform tests.
Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
---
Changes for v3:
- None
Changes for v2:
- Rewrite of "alt_info_env_name" variable description
- Use of get() method on python's dictionary to easily obtain default
value
This patch replaces hardcoded (i.e. 0 and 1) values passed to dfu_{read|write}
with variables.
Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
---
Changes for v3:
- Replace per module global variables with ones defined inside a function
Changes for v2:
- None
When implementing test/py hook scripts, it's helpful to read some working
examples. Provide a link to some. The link was mentioned in the commit
message which first added test/py, but not in any documentation file.
Suggested-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
For example this setting:
env__net_tftp_readable_file = {
"fn": "ep108/image.ub",
"addr": 0x10000000,
"size": 25846296,
"crc32": "b726f9de",
}
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
This shows more output, such as the internal output generated by the unit
test ("ut") command, which makes it easier to debug issues.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
test_hush_if_test.py executes commands that require large values of
CONFIG_SYS_MAXARGS. Detect cases where the configured value is too low
and skip those tests.
Ideally, this would be implemented inside console.run_command(). However,
the command passed to that function is already a completely formed string,
and determining its argument count usage would require splitting commands
at ;, handling quoting to deal with arguments containing spaces, etc. Even
passing the command as a list wouldn't solve all these issues, since we'd
still need to split commands on ; and deal with cases like "if test ..."
which consumes 0 of the argument count.
Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
Acked-by: Simon Glass <sjg@chromium.org>
Sending CTRL-C to QEMU's stdin aborts the process, even if stdin is being
used as a serial port (at least in the raspi2 machine with "qemu -serial
stdin"). Avoid sending CTRL-C to U-Boot to prevent it exiting.
I'd originally used CTRL-C to make sure that if the character used to
abort autoboot ended up being treated as part of a command as well, it'd
abort command entry and return the prompt to a known state. However, this
is not needed, since aborting the autoboot eats the character used to do
that.
Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
Acked-by: Simon Glass <sjg@chromium.org>
Provide user option to skip SPL signature verification for cases where
u-boot is build with SPL support but full U-Boot is also verified
without SPL.
If you want to support this feature please add env__spl_skipped = True
to your boardenv configuration file.
For example Xilinx Zynq is using this feature where the same U-Boot
binary is checked with SPL and without SPL(with FSBL).
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
check for U-Boot SPL signature only if SPL really has a serial output.
So check if CONFIG_SPL_SERIAL_SUPPORT is active in board config.
Signed-off-by: Heiko Schocher <hs@denx.de>
Tested-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
The initial boot of U-Boot happens within the context of the first test
that needs to access the U-Boot console when there is no existing
connection. This keeps all activity nestled within test execution, which
fits well into the pytest model. However, this mingles the U-Boot startup
logs with the execution of some test(s), which hides find the boundary
between the two.
To solve this, wrap the "Starting U-Boot" logic into a separate log
section. If the user wishes, they can simply collapse this log section
when viewing the HTML log, to concentrate purely on the test's own
interaction.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
u_boot_console.exec_attach.get_spawn() performs two steps:
1) Spawn a process to communicate with the serial console.
2) Reset the board so that U-Boot starts running from scratch.
Currently, if an exception happens in step (2), no cleanup is performed on
the process created in step (1). That process stays running and may e.g.
hold serial port locks, or simply continue to read data from the serial
port, thus preventing it from reaching any other process that attempts to
read from the same serial port later. While there is error cleanup code in
u_boot_console_base.ensure_spawned(), this is not triggered since the
exception prevents assignment to self.p there, and hence the exception
handler has no object to operate upon in cleanup_spawn().
Solve this by enhancing u_boot_console.exec_attach.get_spawn() to clean
up any objects it has created.
In theory, u_boot_spawn.Spawn's constructor has a similar issue, so fix
this too.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Use lists rather than sets to record the status of tests. This causes
the test summary in the HTML file to be generated in the same order as
the tests are (or would have been) run. This makes it easier to locate
the first failed test. The log for this test might have interesting
first clues re: interaction with the environment (e.g. hardware flashing,
serial console, ...) and may help tracking down external issues.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
The Python ini file parser that's used to parse .config converts all keys
to lower-case. Hence, all queries against the results must use lower-case.
Fix u_boot_console.ensure_spawned() to test CONFIG_SPL correctly, or the
connection will fail for boards that have SPL.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
The code replaced pexpect with custom code long ago. Don't import the
unused module so it doesn't need to be installed.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Add documentation describing the new --gdbserver feature, and some common
pytest options.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Invoke each "ut"-based unit test as a separate pytest.
Now that the DM unit test runs under test/py, remove the manual shell
script that invokes it.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org> # v2, on sandbox
The existing regex simply ensures that the captured version string doesn't
go past the end of a line. We really want to grab as much as possible. Do
this by explicitly including a ) character at the end of the regex to
match the last character of the version test.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
A regex match object's .end() value is already the index after the match,
not the index of the last character in the match, so there's no need to
add 1 to point past the match.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Implement three improvements to the HTML log file:
- Ability to expand/contract sections. All passing sections are contracted
at file load time so the user can concentrate on issues requiring
action.
- The overall status report is copied to the top of the log for easy
access.
- Add links from the status report to the test logs, for easy navigation.
This all relies on Javascript and the jquery library. If the user doesn't
have Javascript enabled, or jquery can't be downloaded, the log should
look and behave identically to how it did before this patch.
A few notes on the diff:
- A few more 'with log.section("xxx")' were added, so that all stream
blocks are kept within a section block for consistent HTML entity
nesting structure. This changed indentation in a few places, making
the diff look slightly larger.
- HTML entity IDs are cleaned up. We assign simple incrementing integer
IDs now, rather than using mangled test names which were possibly
invalid.
- Sections and streams now use common CSS class names (in addition to the
current separate class names) to more easily share the new behaviour.
This also reduces the CSS file size since rules don't need to be
duplicated.
- An "OK" status is logged after some external command executions so that
make and flash steps are auto-contracted at log file load time, assuming
they passed.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
The test/py/test.py wrapper script catches exceptions thrown when
exec()ing py.test in order to print a helpful error message. However,
the exception handling code squashes the exception and so the script
exits with a non-zero exit code, leading callers to believe that it
passed. Fix this.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Implement command--line option --gdbserver COMM, which does two things:
a) Run the sandbox process under gdbserver, using COMM as gdbserver's
communication channel.
b) Disables all timeouts, so that if U-Boot is halted under the debugger,
tests don't fail. If the user gives up in the middle of a debugging
session, they can simply CTRL-C the test script to abort it.
This allows easy debugging of test failures without having to manually
re-create the failure conditions. Usage is:
Window 1:
./test/py/test.py --bd sandbox --gdbserver localhost:1234
Window 2:
gdb ./build-sandbox/u-boot -ex 'target remote localhost:1234'
When using this option, it likely makes sense to use pytest's -k option
to limit the set of tests that are executed.
Simply running U-Boot directly under gdb (rather than gdbserver) was
also considered. However, this was rejected because:
a) gdb's output would then be processed by the test script, and likely
confuse it causing false failures.
b) pytest by default hides stdout from tests, which would prevent the
user from interacting with gdb.
While gdb can be told to redirect the debugee's stdio to a separate
PTY, this would appear to leave gdb's stdio directed at the test
scripts and the debugee's stdio directed elsewhere, which is the
opposite of the desired effect. Perhaps some complicated PTY muxing
and process hierarchy could invert this. However, the current scheme
is simple to implement and use, so it doesn't seem worth complicating
matters.
c) Using gdbserver allows arbitrary debuggers to be used, even those with
a GUI. If the test scripts invoked the debugger themselves, they'd have
to know how to execute arbitary applications. While the user could hide
this all in a wrapper script, this feels like extra complication.
An interesting future idea might be a --gdb-screen option, which could
spawn both U-Boot and gdb separately, and spawn the screen into a newly
created window under screen. Similar options could be envisaged for
creating a new xterm/... too.
--gdbserver currently only supports sandbox, and not real hardware.
That's primarily because the test hooks are responsible for all aspects of
hardware control, so there's nothing for the test scripts themselves can
do to enable gdbserver on real hardware. We might consider introducing a
separate --disable-timeouts option to support use of debuggers on real
hardware, and having --gdbserver imply that option.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Allow the env__dfu_configs boardenv data to specify the set of DFU
transfer sizes to test. Manually specifying test sizes is useful if you
wish to test multiple DFU configurations (e.g. SD card ext4 filesystem, SD
card whole raw partition, RAM, etc.), but don't want to test every
single transfer size on each, to avoid bloating the overall time taken by
testing. If the boardenv doesn't specify a set of sizes, the built-in list
is used as a default, preserving backwards-compatibility.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Some unit tests expect the cwd of the sandbox process to be the root
of the source tree. Ensure that requirement is met.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
This is required for at least "ut dm" to operate correctly.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Tests can complete in passed, skipped, xpass, xfailed, or failed, states.
Currently the U-Boot log generation code doesn't handle the xfailed or
xpass states since they aren't used. Add support for the remaining states.
Without this, tests that xfail end up being reported as skipped.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Many error situations in U-Boot print the message:
### ERROR ### Please RESET the board ###
Add this to the list of bad patterns the test system detects. One
practical advantage of this change is to detect the case where sandbox
is told to use a particular DTB file, and the file cannot be opened.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Currently, bad patterns are only honored when executing a shell command.
Other cases, such as the initial boot-up of U-Boot or when interacting
with command output rather than gathering all output prior to the shell
prompt, do not currently look for bad patterns in console output. This
patch makes sure that bad patterns are honored everywhere.
One benefit of this change is that if U-Boot sandbox fails to start up,
the error message it emits can be caught immediately, rather than relying
on a (long) timeout when waiting for the expected signon message and/or
command prompt.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
A future patch will use the bad_patterns array in multiple places. Rather
than duplicating the code to calculate it, or even sharing it in a
function and simply calling it redundantly when nothing has changed, only
re-calculate the list when some change is made to it. This reduces work.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Multiple patterns may be passed to spawn.expect(). The pattern which
matches at the earliest position should be designated as the match. This
aspect works correctly. When multiple patterns match at the same position,
priority should be given the the earliest entry in the list of patterns.
This aspect does not work correctly. This patch fixes it.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
When pytest generates the name for parametrized tests, simple parameter
values (ints, strings) get used directly, but more complex values such
as dicts are not handled. This yields test names such as:
dfu[env__usb_dev_port0-env__dfu_config0]
dfu[env__usb_dev_port0-env__dfu_config1]
Add some code to extract a custom fixture ID from the fixture values, so
that we end up with meaningful names such as:
dfu[micro_b-emmc]
dfu[devport2-ram]
If the boardenv file doesn't define custom names, the code falls back to
the old algorithm.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
When converting test/py from " to ', I missed a few places (or added a
few inconsistencies later). Fix these.
Note that only quotes in code are converted; double-quotes in comments
and HTML are left as-is, since English and HTML use " not '.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Python's coding style docs indicate to use " not ' for docstrings.
test/py has other violations of the coding style docs, since the docs
specify a stranger style than I would expect, but nobody has complained
about those yet:-)
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
The existing net test executes a list of commands supplied by boardenv
variable env__net_pre_commands. The idea was that boardenv would know
whether the Ethernet device was attached to USB, PCI, ... and hence was
the best place to put any commands required to probe the device.
However, this approach doesn't scale well when attempting to use a single
boardenv across multiple branches of U-Boot, some of which require "pci
enum" to enumerate PCI and others of which don't, or don't /yet/ simply
because various upstream changes haven't been merged down.
This patch updates the test to require that the boardenv state which HW
features are required for Ethernet to work, and lets the test itself map
that knowledge to the set of commands to execute. Since this mapping is
part of the test script, which is part of the U-Boot code/branch, this
approach is more scalable. It also feels cleaner, since again boardenv
is only providing data, rather than test logic.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
The DFU test requests U-Boot configure its USB controller in device mode,
then waits for the host machine to enumerate the USB device and create a
device node for it. However, this wait can be fooled if the USB device
node already exists before the test starts, e.g. if some previous software
stack already configured the USB controller into device mode and never
de-configured it. This "previous software stack" could even be another
test/py test, if U-Boot's own USB teardown does not operate correctly. If
this happens, dfu-util may be run before U-Boot is ready to serve DFU
commands, which may cause false test failures.
Enhance the dfu test to fail if the device node exists before it is
expected to.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
test/py contains logic to detect the target crashing and rebooting by
searching the console output for a U-Boot signon message, which will
presumably be emitted when the system boots after the crash/reset.
Currently, this logic only searches for the exact signon message that
was printed by the U-Boot version under test, upon the assumption that
binary is written into flash, and hence will be the version booted after
any reset. However, this is not a valid assumption; some test setups
download the U-Boot-under-test into RAM and boot it from there, and in
such a scenario an arbitrary U-Boot version may be located in flash and
hence run after any reset.
Fix the reset detection logic to match any U-Boot signon message. This
prevents false negatives.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
find_ram_base() is a shared utility function, not a core part of the
U-Boot console interaction.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Add a test of DFU functionality to the Python test suite. The test
starts DFU in U-Boot, waits for USB device enumeration on the host,
executes dfu-util multiple times to test various transfer sizes, many
of which trigger USB driver edge cases, and finally aborts the DFU
command in U-Boot.
This test mirrors the functionality previously available via the shell
scripts in test/dfu, and hence those are removed too.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Acked-by: Simon Glass <sjg@chromium.org>
Enhance the UMS test to optionally mount a partition and read/write a file
to it, validating that the content written and read back are identical.
This enhancement is backwards-compatible; old boardenv contents that don't
define the new configuration data will cause the test code to perform as
before.
test/ums/ is deleted since the Python test now performs the same testing
that it did.
The code is also re-written to make use of the recently added utility
module, and split it up into nested functions so the overall logic of
the test process can be followed more easily without the details
cluttering the code.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Acked-by: Simon Glass <sjg@chromium.org>
Add various common utility functions. These will be used by a forthcoming
re-written UMS test, and a brand-new DFU test.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Sometimes it's useful to run shell commands and ignore any errors. One
example might be cleanup logic; if a test-case experiences an error, the
cleanup logic might experience an error too, and we don't want that error
to mask the original error, so we want to ignore the subsequent error.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Write a note to the log file when a test sends CTRL-C to U-Boot. This
makes it easier to follow what's happening in the logs, especially since
U-Boot doesn't echo the character back to its output, so there's no other
signal of what's going on.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Tests may fail for a number of reasons, and in particular for reasons
other than a timeout waiting for U-Boot to print expected data. If the
last operation that a failed test performs is not waiting for U-Boot to
print something, then any trailing output from U-Boot during that test's
operation will not be logged as part of that test, but rather either
along with the next test, or even thrown away, potentiall hiding clues
re: the test failure reason.
Solve this by explicitly draining (and hence logging) the U-Boot output
in the case of failed tests.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Prior to this change, U-Boot was lazilly (re-)spawned if/when a test
attempted to interact with it, and no active connection existed. This
approach was simple, yet had the disadvantage that U-Boot might be
spawned in the middle of a test function, e.g. after the test had already
performed actions such as creating data files, etc. In that case, this
could cause the log to contain the sequence (1) some test logs, (2)
U-Boot's boot process, (3) the rest of that test's logs. This isn't
optimally readable. This issue will affect the upcoming DFU and enhanced
UMS tests.
This change converts u_boot_console to be a function-scoped fixture, so
that pytest attempts to re-create the object for each test invocation.
This allows the fixture factory function to ensure that U-Boot is spawned
prior to every test. In practice, the same object is returned each time
so there is essentially no additional overhead due to this change.
This allows us to remove:
- The explicit ensure_spawned() call from test_sleep, since the core now
ensures that the spawn happens before the test code is executed.
- The laxy calls to ensure_spawned() in the u_boot_console_*
implementations.
The one downside is that test_env's "state_ttest_env" fixture must be
converted to a function-scoped fixture too, since a module-scoped fixture
cannot use a function-scoped fixture. To avoid overhead, we use the same
trick of returning the same object each time.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Currently, Spawn.expect() imposes its timeout solely upon receipt of new
data, not on its overall operation. In theory, this could cause the
timeout not to fire if U-Boot continually generated output that did not
match the expected patterns.
Fix the code to additionally impose a timeout on overall operation, which
is the intended mode of operation.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Lukasz Majewski <l.majewski@samsung.com>
Acked-by: Simon Glass <sjg@chromium.org>
Execute "sleep", and validate that it sleeps for approximately the correct
amount of time.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
This test invokes the "ums" command in U-Boot, and validates that a USB
storage device is enumerated on the test host system, and can be read
from.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Migrate all most tests from command_ut.c into the Python test system.
This allows the tests to be run against any U-Boot binary that supports
the if command (i.e. where hush is enabled) without requiring that
binary to be permanently bloated with the code from command_ut.
Some tests in command_ut.c can only be executed from C code, since they
test internal (more unit-level) features of various U-Boot APIs. The
migrated tests can all operate directly from the U-Boot console.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
This tests whether the following features of the U-Boot shell:
- Execution of a directly entered command.
- Compound commands (; delimiter).
- Quoting of arguments containing spaces.
- Executing commands from environment variables.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
This tests whether md/mw work, and affect each-other.
Command repeat is also tested.
test/cmd_repeat.sh is removed, since the new Python-based test does
everything it used to.
Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
This tests basic environment variable functionality.
Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Test the sandbox port's implementation of the reset command and SIGHUP
handling. These should both cause the U-Boot process to exit gracefully.
Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
This tool aims to test U-Boot by executing U-Boot shell commands using the
console interface. A single top-level script exists to execute or attach
to the U-Boot console, run the entire script of tests against it, and
summarize the results. Advantages of this approach are:
- Testing is performed in the same way a user or script would interact
with U-Boot; there can be no disconnect.
- There is no need to write or embed test-related code into U-Boot itself.
It is asserted that writing test-related code in Python is simpler and
more flexible that writing it all in C.
- It is reasonably simple to interact with U-Boot in this way.
A few simple tests are provided as examples. Soon, we should convert as
many as possible of the other tests in test/* and test/cmd_ut.c too.
The hook scripts, relay control utilities, and udev rules I use for my
own HW setup are published at https://github.com/swarren/uboot-test-hooks.
See README.md for more details!
Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Michal Simek <michal.simek@xilinx.com>
Tested-by: Simon Glass <sjg@chromium.org>
Acked-by: Simon Glass <sjg@chromium.org> #v3