Unified tests into final step ✅ #192
Conversation
|
Here we can see a failed test: It will cause the final action to be skipped, not allowing the user to merge it. |
|
/merge |
|
Only the following action should be required: 
https://github.com/polkadot-fellows/runtimes/actions/runs/7962468177/job/21737338698?pr=192 |
|
Enabled Available commands
For more information see the documentation |
Head branch was pushed to by a user without write access
|
/merge |
|
Enabled Available commands
For more information see the documentation |
Head branch was pushed to by a user without write access
|
I just added the same process to migrations: |
Applied the same technique that I applied in polkadot-fellows/runtimes#192
Applied the same technique that I applied in polkadot-fellows/runtimes#192 to have one final test job (and only depend this one)
Applied the same technique that I applied in polkadot-fellows/runtimes#192 to have one final test job (and only depend this one)
|
/merge |
|
Enabled Available commands
For more information see the documentation |
Applied the same technique that I applied in polkadot-fellows/runtimes#192 to have one final test job (and only depend this one)
Failed to update PR ❌There was an error while trying to keep this PR You may have conflicts More info in the logs 📋 |
|
The problem with this is that we currently have removed the encointer migration check as requirement for merging. How could we achieve the same with this job? |
Uh that will be a problem. This job would ensure that all the tests pass first, including the encointer one. We would have to remove it from the required tests. A solution would be to remove it, and then make a new PR trying to fix it (I assume that you don't want to have broken tests in your pipeline) but that's a subpar suggestion from me. If you prefer I can also disable the required check for migrations and leave only the one for tests |
Failed to update PR ❌There was an error while trying to keep this PR You may have conflicts More info in the logs 📋 |
We kept the test for now to remember us about the test. Currently it is just removed from the list of required tests. I assume fetching the list of required tests for the "all X passed" test would be too complicated? |
Failed to update PR ❌There was an error while trying to keep this PR You may have conflicts More info in the logs 📋 |
Bullrich
force-pushed
the
bullrich/unify-jobs
branch
from
52ecca3 to
870ffb0
Compare
Bullrich
force-pushed
the
bullrich/unify-jobs
branch
from
9597e8c to
307c94f
Compare
Bullrich
force-pushed
the
bullrich/unify-jobs
branch
from
307c94f to
7b28b11
Compare
Bullrich
force-pushed
the
bullrich/unify-jobs
branch
from
7b28b11 to
67c63a9
Compare
|
@Bullrich I think we can merge this now 🙈 |
Bullrich
force-pushed
the
bullrich/unify-jobs
branch
2 times, most recently
from
0e6edb2 to
e5e8483
Compare
Bullrich
force-pushed
the
bullrich/unify-jobs
branch
from
e5e8483 to
53cebda
Compare
Test was singular, it should be plural
The matrix jobs are already depended by other jobs. We can simply remove their dependency
It looks a bit better than simply adding a print line statement
Let's unify everything at once
Added a shorter name to make it easier to understand
Bullrich
force-pushed
the
bullrich/unify-jobs
branch
from
53cebda to
c25ac66
Compare
|
/merge |
|
Enabled Available commands
For more information see the documentation |
fellowship-merge-bot
bot
merged commit 750aa73
into
polkadot-fellows:main
Unified all the tests into having one final step at the end.
This step will only run if all the previous steps have ran. If any of these tests fail, it will fail.
All the status checks will still appear in the PR, so the user can see any failed case:
But, by adding this last test as the only requirement, the list to block the branch becomes significantly smaller.