Skip to content

Conversation

@david-crespo
Copy link
Contributor

@david-crespo david-crespo commented Jan 10, 2022

Followup to #581 to include NetworkInterface change I left out because I couldn't get it to work.

Turns out NetworkInterface wasn't working because I forgot to update the sled agent OpenAPI spec. Presumably there was a failure of test_sled_agent_openapi_sled (was able to get a failure when I ran that test locally), but it must have gotten buried cargo test output, so I missed it.

@david-crespo david-crespo requested a review from smklein January 10, 2022 21:17
@davepacheco
Copy link
Collaborator

Hmm. How did it not break in CI on commit e8c31ef?

@david-crespo
Copy link
Contributor Author

That one didn't have the NetworkInterface change in it. Let me see if I can find a run that did.

@david-crespo
Copy link
Contributor Author

david-crespo commented Jan 10, 2022

Here's a run that should have had a failure for test_sled_agent_openapi_sled but does not.

https://github.com/oxidecomputer/omicron/runs/4744873353?check_suite_focus=true

Only these much harder to debug failures turned up:

 failures:
    integration_tests::disks::test_disks
    integration_tests::instances::test_instances_create_reboot_halt
    integration_tests::instances::test_instances_delete_fails_when_running_succeeds_when_stopped
    integration_tests::subnet_allocation::test_subnet_allocation

@davepacheco
Copy link
Collaborator

Sorry -- I think I misread the issue here. I thought you were saying that the change for #581 had neglected to update the sled agent spec and as a result was causing a test failure on "main".

@david-crespo
Copy link
Contributor Author

Right, sorry. Initially that was true in that PR, but I fixed it there. Will update the description.

@david-crespo
Copy link
Contributor Author

@davepacheco thinks cargo test will bail on running other targets if one target fails. So in this case the integration tests failed, and the sled agent tests that would have alerted me to my mistake did not run.

@david-crespo david-crespo enabled auto-merge (squash) January 10, 2022 22:04
@david-crespo david-crespo requested review from smklein and removed request for smklein January 11, 2022 04:10
@david-crespo david-crespo merged commit b3940c4 into main Jan 11, 2022
@david-crespo david-crespo deleted the flatten-identity-again branch January 11, 2022 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants