Run Mixed Mode Tests during the release parallel to the other tests#3233
Run Mixed Mode Tests during the release parallel to the other tests#3233alecgrieser merged 55 commits intoFoundationDB:mainfrom
Conversation
This is needed for it to be able to find the actions
it needs to be a key-value pair
This avoids potential issues with bash trying to treat the contents as something they're not
The documentation definitely implies that you can use a file, but
when I did it failed with:
tar: mixed-mode-results.md: Cannot open: Not a directory
And it appeared to be running:
```
echo ::group::Archive artifact
tar \
--dereference --hard-dereference \
--directory "$INPUT_PATH" \
-cvf "$RUNNER_TEMP/artifact.tar" \
--exclude=.git \
--exclude=.github \
.
echo ::endgroup::
```
It failed with the cryptic: ``` Prepare all required actions Getting action download info Error: Missing download info for actions/download-artifact@v2 ``` Before; I'm guessing this needs to be at the workflow level for it to be visible.
The docs lead me to believe that I shouldn't have to do this, but it looks like I do. I also renamed things to hopefully make things nicer.
Rather than inserting a placeholder and then updating
Accidentally deleted that when deleting logging
I had to leave the file around because github won't let you run workflows that don't have the workflow on main, but it doesn't have be the same, so it just echos a message if you try to run the new one on a newer version
I don't think there was any reason to move this down, and it was commented out during my manual testing
ScottDugas
force-pushed
the
parallel-release-build
branch
from
bcbbbe5 to
de3dd17
Compare
| if (context.ref == "refs/heads/main") { | ||
| return "BUILD"; | ||
| } else if (context.ref.startsWith("refs/heads/")) { | ||
| return "PATCH"; |
There was a problem hiding this comment.
One consequence of this approach is that if someone runs a patch build, they use the patch process that is baked into their branch, rather than the one on main. In some ways, that's good, because it means that if there's a relatively stable release process in a past branch, then past builds will continue to use that. But it also means that we need to be a little mindful of keeping past patch branches up to date if there are any changes that need to be reflected everywhere. Overall, this is probably fine, and there was actually already some dependency between the different branches (as any of the composite actions would borrow the version from the branch), but I did want to raise this nonetheless
There was a problem hiding this comment.
Yes, It used to be that you could select a different branch for the workflow, vs the branch that you are releasing.
So that was strictly more flexible.
As you note, the composite actions would always chose from the branch being released.
We could definitely add a branch input that allows you to use the workflow from one branch, and release a different branch, but it seems rather delicate, and we'd be better off not doing it right now, and seeing how a branch release goes. We can always backport any workflow changes to the other branch.
There was a problem hiding this comment.
It also kind of looks like my patch release may be messing with the workflows in general, which seems super weird.
It, all of a sudden, has 5 runs, and all of them say it's invalid: https://github.com/FoundationDB/fdb-record-layer/actions/workflows/patch_release.yml
I fixed the issue (I think), but nothing happened after that.
I verified that it does use the same hash, and tested it by changing the `gradle.properties` while the test was running, and publish checked out the old commit, and failed to push.
upload-pages-artifacts was definitely a mistake, hopefully this means that download won't be downloading a tar.
These were only used in one place, and separating them out didn't end up making the release workflow file much simpler
the code to inject them still exists, but I don't see us using this in the future, as we will always be adding the results when we add the release to the release notes
Rather than having it dump the contents to stdout, and piping it to a file. This will allow us to potentially have logs of some sort to stdout.
I don't know why this worked before, but not when it was inlined, maybe I missed something, but it pretty clearly errored noting that I need to put this in the env.
alecgrieser
left a comment
alecgrieser
left a comment
There was a problem hiding this comment.
This mostly LGTM. There's one last thing regarding mixed-mode-results.yml to make sure we don't accidentally include it in the PR that we (optionally) generate.
| with: | ||
| name: mixed-mode-results | ||
| path: ./artifact | ||
| path: mixed-mode-results.md |
There was a problem hiding this comment.
We should consider adding mixed-mode-results.md to the .gitignore, or placing it in a directory that is already excluded. The way the PR creating workflow works, if there are files that aren't in the .gitignore present, it will include them in the PR that it creates. Excluding the file would make sure that we don't accidentally do that, particularly in the other step when we download it.
This will ensure that the release process doesn't accidentally commit this file, or add it to a PR. It may make sense to move this to .out, but I didn't want it to get messed with by other parts of the release flow that use .out or have to add another mv step.
2 participants
This changes our release build to run the MixedModeTest in parallel with the other tests so that it can populate the release notes at that time. One key advantage of this is the documentation that sphinx publishes will have this information.
This also changes the release build so that it is usable for both patch and non-patch builds, and figures that out depending on the target branch.
Note: there are a lot of commits here, I don't think I recommend going through this change commit-by-commit, there was a lot of trial-and-error to get here.