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.

Sign up for GitHub

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

Jump to bottom

ci: add test apps to `build-boards.yml` #1667

Merged

hawkw merged 9 commits into master from eliza/build-kernel-tests

Mar 20, 2024

Member

hawkw commented Mar 19, 2024

Currently, Hubris' CI doesn't build the tests at all. This means that it's quite easy for the various test apps to bitrot --- for example, the PSC test app doesn't build, because it maps IRQs by bit, rather than by name. Named IRQ support was added back in 2022 (e.g. #424), so this TOML clearly hasn't been touched in...kind of a while, which I think is a pretty convincing argument for building these tests on CI!

hawkw requested a review from labbott

March 19, 2024 18:13

hawkw added a commit that referenced this pull request


          ci: add test apps to build-boards.yml (#1667)

ba673fd

Currently, Hubris' CI doesn't build the tests at all. This means that
it's quite easy for the various test apps to bitrot --- for example, the
PSC test app doesn't build, because [it maps IRQs by bit, rather than by
name][1]. Named IRQ support was added back in 2022 (e.g. #424), so this
TOML clearly hasn't been touched in...kind of a while, which I think is
a pretty convincing argument for building these tests on CI!

[1]: https://github.com/oxidecomputer/hubris/blob/3325c6343a3e51a4d2c63cb3eac46085ba61dac3/test/tests-psc/app.toml#L68-L74

hawkw force-pushed the eliza/build-kernel-tests branch from d840353 to ba673fd Compare

March 19, 2024 18:14

Member Author

hawkw commented Mar 19, 2024

Trying to update the bitrotted PSC test config to actually build, but there's a bit of an unfortunate wrinkle: the app config for the PSC tests include a drv-stm32xx-i2c-server (which we need for the FRU ID test), which then depends on drv-stm32xx-sys, which depends on a task slot for jefe...but, because we use test-runner as the supervisor for the test suite, we don't have a jefe!

I think getting these tests to build again will require an actually meaningful amount of work --- perhaps we can add a feature to drv-stm32xx-sys to make it play nice with test-runner as supervisor (and add code to the test runner to allow it to do whatever Jefe stuff that the sys driver depends on)?

hawkw added a commit that referenced this pull request


          test: fix bitrotted tests-psc config (#1667)

eed879f

This ended up being a bit more complex, as the `tests-psc` config
includes a `drv-stm32xx-i2c` task, which depends on `drv-stm32xx-sys`,
which depends on `jefe`. Because the test runner runs as supervisor,
there _is_ no `jefe`. Therefore, I've added a feature to
`drv-stm32xx-sys` to disable its `jefe` task slot, for use in tests.

It would probably be more ideologically correct to make the
`task-jefe-api` an optional dep, nand only talk to Jefe if it's enabled.
But, IIRC, we never enable default features by default, so making Jefe
support opt-in rather than opt-out would require changing _every other
app.toml with a `drv-stm32xx-sys` task_ to enable the Jefe feature. I
dunno...

hawkw added a commit that referenced this pull request


          test: fix bitrotted tests-psc config (#1667)

2f03459

This ended up being a bit more complex, as the `tests-psc` config
includes a `drv-stm32xx-i2c` task, which depends on `drv-stm32xx-sys`,
which depends on `jefe`. Because the test runner runs as supervisor,
there _is_ no `jefe`. Therefore, I've added a feature to
`drv-stm32xx-sys` to disable its `jefe` task slot, for use in tests.

It would probably be more ideologically correct to make the
`task-jefe-api` an optional dep, nand only talk to Jefe if it's enabled.
But, IIRC, we never enable default features by default, so making Jefe
support opt-in rather than opt-out would require changing _every other
app.toml with a `drv-stm32xx-sys` task_ to enable the Jefe feature. I
dunno...

hawkw force-pushed the eliza/build-kernel-tests branch from eed879f to 2f03459 Compare

March 19, 2024 19:02

hawkw added a commit that referenced this pull request


          ci: add test apps to build-boards.yml (#1667)

6c3fa90

Currently, Hubris' CI doesn't build the tests at all. This means that
it's quite easy for the various test apps to bitrot --- for example, the
PSC test app doesn't build, because [it maps IRQs by bit, rather than by
name][1]. Named IRQ support was added back in 2022 (e.g. #424), so this
TOML clearly hasn't been touched in...kind of a while, which I think is
a pretty convincing argument for building these tests on CI!

[1]: https://github.com/oxidecomputer/hubris/blob/3325c6343a3e51a4d2c63cb3eac46085ba61dac3/test/tests-psc/app.toml#L68-L74

hawkw force-pushed the eliza/build-kernel-tests branch from 4949604 to f89f20e Compare

March 19, 2024 21:33

hawkw added a commit that referenced this pull request


          test: fix bitrotted tests-psc config (#1667)

945267b

This ended up being a bit more complex, as the `tests-psc` config
includes a `drv-stm32xx-i2c` task, which depends on `drv-stm32xx-sys`,
which depends on `jefe`. Because the test runner runs as supervisor,
there _is_ no `jefe`. Therefore, I've added a feature to
`drv-stm32xx-sys` to disable its `jefe` task slot, for use in tests.

It would probably be more ideologically correct to make the
`task-jefe-api` an optional dep, nand only talk to Jefe if it's enabled.
But, IIRC, we never enable default features by default, so making Jefe
support opt-in rather than opt-out would require changing _every other
app.toml with a `drv-stm32xx-sys` task_ to enable the Jefe feature. I
dunno...

hawkw added a commit that referenced this pull request


          test: embiggen g070 kernel (#1667)

f89f20e

hawkw added a commit that referenced this pull request


          ci: add test apps to build-boards.yml (#1667)

949139c

Currently, Hubris' CI doesn't build the tests at all. This means that
it's quite easy for the various test apps to bitrot --- for example, the
PSC test app doesn't build, because [it maps IRQs by bit, rather than by
name][1]. Named IRQ support was added back in 2022 (e.g. #424), so this
TOML clearly hasn't been touched in...kind of a while, which I think is
a pretty convincing argument for building these tests on CI!

[1]: https://github.com/oxidecomputer/hubris/blob/3325c6343a3e51a4d2c63cb3eac46085ba61dac3/test/tests-psc/app.toml#L68-L74

hawkw added a commit that referenced this pull request


          test: fix bitrotted tests-psc config (#1667)

a031e66

This ended up being a bit more complex, as the `tests-psc` config
includes a `drv-stm32xx-i2c` task, which depends on `drv-stm32xx-sys`,
which depends on `jefe`. Because the test runner runs as supervisor,
there _is_ no `jefe`. Therefore, I've added a feature to
`drv-stm32xx-sys` to disable its `jefe` task slot, for use in tests.

It would probably be more ideologically correct to make the
`task-jefe-api` an optional dep, nand only talk to Jefe if it's enabled.
But, IIRC, we never enable default features by default, so making Jefe
support opt-in rather than opt-out would require changing _every other
app.toml with a `drv-stm32xx-sys` task_ to enable the Jefe feature. I
dunno...

hawkw added a commit that referenced this pull request


          test: embiggen g070 kernel (#1667)

731db63

hawkw force-pushed the eliza/build-kernel-tests branch from f89f20e to 731db63 Compare

March 19, 2024 21:43

hawkw added a commit that referenced this pull request


          ci: add test apps to build-boards.yml (#1667)

8560d23

Currently, Hubris' CI doesn't build the tests at all. This means that
it's quite easy for the various test apps to bitrot --- for example, the
PSC test app doesn't build, because [it maps IRQs by bit, rather than by
name][1]. Named IRQ support was added back in 2022 (e.g. #424), so this
TOML clearly hasn't been touched in...kind of a while, which I think is
a pretty convincing argument for building these tests on CI!

[1]: https://github.com/oxidecomputer/hubris/blob/3325c6343a3e51a4d2c63cb3eac46085ba61dac3/test/tests-psc/app.toml#L68-L74

hawkw added a commit that referenced this pull request


          test: fix bitrotted tests-psc config (#1667)

15ff609

This ended up being a bit more complex, as the `tests-psc` config
includes a `drv-stm32xx-i2c` task, which depends on `drv-stm32xx-sys`,
which depends on `jefe`. Because the test runner runs as supervisor,
there _is_ no `jefe`. Therefore, I've added a feature to
`drv-stm32xx-sys` to disable its `jefe` task slot, for use in tests.

It would probably be more ideologically correct to make the
`task-jefe-api` an optional dep, nand only talk to Jefe if it's enabled.
But, IIRC, we never enable default features by default, so making Jefe
support opt-in rather than opt-out would require changing _every other
app.toml with a `drv-stm32xx-sys` task_ to enable the Jefe feature. I
dunno...

hawkw added a commit that referenced this pull request


          test: embiggen g070 kernel (#1667)

b8f0cab

hawkw force-pushed the eliza/build-kernel-tests branch from 731db63 to b8f0cab Compare

March 19, 2024 22:09

hawkw added 4 commits

March 19, 2024 15:56


          ci: add test apps to build-boards.yml (#1667)

5772fba

Currently, Hubris' CI doesn't build the tests at all. This means that
it's quite easy for the various test apps to bitrot --- for example, the
PSC test app doesn't build, because [it maps IRQs by bit, rather than by
name][1]. Named IRQ support was added back in 2022 (e.g. #424), so this
TOML clearly hasn't been touched in...kind of a while, which I think is
a pretty convincing argument for building these tests on CI!

[1]: https://github.com/oxidecomputer/hubris/blob/3325c6343a3e51a4d2c63cb3eac46085ba61dac3/test/tests-psc/app.toml#L68-L74


          test: fix bitrotted tests-psc config (#1667)

This ended up being a bit more complex, as the `tests-psc` config
includes a `drv-stm32xx-i2c` task, which depends on `drv-stm32xx-sys`,
which depends on `jefe`. Because the test runner runs as supervisor,
there _is_ no `jefe`. Therefore, I've added a feature to
`drv-stm32xx-sys` to disable its `jefe` task slot, for use in tests.

It would probably be more ideologically correct to make the
`task-jefe-api` an optional dep, nand only talk to Jefe if it's enabled.
But, IIRC, we never enable default features by default, so making Jefe
support opt-in rather than opt-out would require changing _every other
app.toml with a `drv-stm32xx-sys` task_ to enable the Jefe feature. I
dunno...


          test: embiggen g070 kernel (#1667)

5485a02


          test: assorted test suite clippy placation (#1667)

d7e262d

Turns out [Clippy dislikes some of the test suite code][1]. This commit
makes Clippy chill out about it.

[1]: https://github.com/oxidecomputer/hubris/actions/runs/8350750156/job/22857850090

hawkw force-pushed the eliza/build-kernel-tests branch from b8f0cab to d7e262d Compare

March 19, 2024 22:56

hawkw requested a review from cbiffle

March 19, 2024 22:56

cbiffle approved these changes

View reviewed changes

Collaborator

cbiffle left a comment

Looks plausible! Thanks for unrotting these.

hawkw added 4 commits

March 19, 2024 16:19


          test: fix clippy lints in test-assist (#1667)

5de206e


          test: fix clippy lints in test-idol-server (#1667)

3bbd2c8


          test: fix clippy lints in hiffy test support (#1667)

7f45374


          test: remove armv6m_atomic_hack (#1667)

10400cc

Turns out this import was totally unused --- there are no atomics in here.


          ci: fix image names for LPC55 (#1667)

e87e9d7

Whoops, these build two images. My bad.

hawkw merged commit 67dd7e6 into master

103 checks passed

hawkw added a commit that referenced this pull request


          ci: add test apps to build-boards.yml (#1667)

388b0d8

Currently, Hubris' CI doesn't build the tests at all. This means that
it's quite easy for the various test apps to bitrot --- for example, the
PSC test app doesn't build, because [it maps IRQs by bit, rather than by
name][1]. Named IRQ support was added back in 2022 (e.g. #424), so this
TOML clearly hasn't been touched in...kind of a while, which I think is
a pretty convincing argument for building these tests on CI!

[1]: https://github.com/oxidecomputer/hubris/blob/3325c6343a3e51a4d2c63cb3eac46085ba61dac3/test/tests-psc/app.toml#L68-L74

hawkw added a commit that referenced this pull request


          test: fix bitrotted tests-psc config (#1667)

e99b877

This ended up being a bit more complex, as the `tests-psc` config
includes a `drv-stm32xx-i2c` task, which depends on `drv-stm32xx-sys`,
which depends on `jefe`. Because the test runner runs as supervisor,
there _is_ no `jefe`. Therefore, I've added a feature to
`drv-stm32xx-sys` to disable its `jefe` task slot, for use in tests.

It would probably be more ideologically correct to make the
`task-jefe-api` an optional dep, nand only talk to Jefe if it's enabled.
But, IIRC, we never enable default features by default, so making Jefe
support opt-in rather than opt-out would require changing _every other
app.toml with a `drv-stm32xx-sys` task_ to enable the Jefe feature. I
dunno...

hawkw added a commit that referenced this pull request


          test: embiggen g070 kernel (#1667)

5e87234

hawkw added a commit that referenced this pull request


          test: assorted test suite clippy placation (#1667)

bd17a56

Turns out [Clippy dislikes some of the test suite code][1]. This commit
makes Clippy chill out about it.

[1]: https://github.com/oxidecomputer/hubris/actions/runs/8350750156/job/22857850090

hawkw added a commit that referenced this pull request


          test: fix clippy lints in test-assist (#1667)

376788f

hawkw added a commit that referenced this pull request


          test: fix clippy lints in test-idol-server (#1667)

6f8b316

hawkw added a commit that referenced this pull request


          test: fix clippy lints in hiffy test support (#1667)

46a1314

hawkw added a commit that referenced this pull request


          test: remove armv6m_atomic_hack (#1667)

66dd89d

Turns out this import was totally unused --- there are no atomics in here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet