Skip to content
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.

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

420 change github workflow to run test with feature flags #488

Merged
merged 38 commits into from
Oct 30, 2024

Conversation

b-yap
Copy link
Contributor

@b-yap b-yap commented Jul 24, 2024

closes #420
Add another step to run a test but with features runtime-benchmarks and try-runtime.

This also removes nightly

@b-yap b-yap force-pushed the 420-change-github-workflow-to-run-tests-with-feature-flags branch from 53ce42c to 7b0a48d Compare September 9, 2024 12:06
@b-yap b-yap changed the base branch from main to polkadot-v1.1.0 September 10, 2024 08:35
@b-yap b-yap marked this pull request as ready for review September 10, 2024 18:29
@b-yap b-yap marked this pull request as draft September 10, 2024 18:35
@b-yap b-yap marked this pull request as ready for review September 11, 2024 06:53
@b-yap b-yap requested a review from a team September 11, 2024 06:54
@ebma
Copy link
Member

ebma commented Sep 11, 2024

Just so I understand properly, you are saying that the change requested in #420 is useless because all tests are filtered out?

Let's maybe change the workflow here to run the checks on any PR, not just on PRs that merge to main.

@b-yap b-yap force-pushed the 420-change-github-workflow-to-run-tests-with-feature-flags branch from bfd9e9d to b5f3ef2 Compare September 12, 2024 06:34
Copy link
Member

@TorstenStueber TorstenStueber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to @ebma's question, to me the intention of #420 is not entirely clear: is the task to 1) just filter out all test for --features runtime-benchmarks or is it to 2) make these tests work also for --features runtime-benchmarks.

The issue says:

This allows for changes that break benchmarks to pass the pipeline and therefore be merged into the main branch

This almost sounds to me like it means option 2.

Cargo.toml Outdated Show resolved Hide resolved
Base automatically changed from polkadot-v1.1.0 to main September 19, 2024 08:36
@b-yap b-yap changed the base branch from main to pendulum-release September 19, 2024 09:03
@b-yap b-yap changed the base branch from pendulum-release to main September 19, 2024 09:03
@b-yap b-yap force-pushed the 420-change-github-workflow-to-run-tests-with-feature-flags branch from 9ef613f to d295174 Compare September 19, 2024 09:15
@ebma
Copy link
Member

ebma commented Oct 10, 2024

Let's wait for #485 to be merged before we merge this one to avoid conflicts with the workflow definitions.

gianfra-t and others added 25 commits October 22, 2024 17:46
```
error[E0046]: not all trait items implemented, missing: `try_successful_origin`
  --> /Users/b.carlayap/.cargo/git/checkouts/cumulus-59522f43471fa161/f603a61/parachains/runtimes/assets/common/src/foreign_creators.rs:28:1
   |
28 | / impl<
29 | |         IsForeign: ContainsPair<MultiLocation, MultiLocation>,
30 | |         AccountOf: Convert<MultiLocation, AccountId>,
31 | |         AccountId: Clone,
...  |
36 | |     RuntimeOrigin::PalletsOrigin:
37 | |         From<XcmOrigin> + TryInto<XcmOrigin, Error = RuntimeOrigin::PalletsOrigin>,
   | |___________________________________________________________________________________^ missing `try_successful_origin` in implementation
   ```
   just yet
…ime`

move the benchmark to another workflow
update CI to 1 file only
@b-yap b-yap force-pushed the 420-change-github-workflow-to-run-tests-with-feature-flags branch from d295174 to 40f0e15 Compare October 22, 2024 13:07
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great to remove the need for stable vs nightly configurations and it's also nice that you extracted the clippy checks from the tests so they can already run in parallel.

Ready for merge IMO 👍

@b-yap b-yap merged commit 72b91f7 into main Oct 30, 2024
3 checks passed
@b-yap b-yap deleted the 420-change-github-workflow-to-run-tests-with-feature-flags branch October 30, 2024 08:15
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.

Change Github workflow to run tests with feature flags
5 participants