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

improve CI tests #157

Closed
wants to merge 7 commits into from
Closed

improve CI tests #157

wants to merge 7 commits into from

Conversation

muraca
Copy link
Collaborator

@muraca muraca commented Nov 30, 2023

This PR introduces a custom action to run tests for different crates, and updates the workflow to use this action for testing:

  • tuxedo-core;
  • tuxedo-parachain-core and parachain-piece, which is currently skipped as it fails because it uses too much disk space;
  • tuxedo-template-runtime, which I think we could skip entirely;
  • all the other wardrobe pieces, which should be updated manually when the list changes.

This PR is a step towards #155.

@JoshOrndorff
Copy link
Contributor

I'm open to going this route if it is necessary. But I have some hesitations.

  1. It adds the maintenance overhead of a custom action. I'd rather stick closely to the simple recipes from https://github.com/actions-rs/example
  2. It is kind of a workaround for the underlying problem which is that our code takes way too fing long to compile. Obviously we inherent this problem from Polkadot, but I'd like to try to improve the compile time first.

@muraca
Copy link
Collaborator Author

muraca commented Dec 10, 2023

It adds the maintenance overhead of a custom action.

True to a certain extent, since it is like a code function we call from multiple sources. It has been made just to avoid copy-pasting the same thing over and over again.

I'd rather stick closely to the simple recipes from actions-rs

Well, actions-rs has been archived and is no longer maintained. But I don't see how it would be relevant for us.
Do you mean we should find an "open source" action that does what we need to (eg test and code coverage using cache) and stick with that? I don't necessarily agree.

I'd like to try to improve the compile time first.

Sorry if this wasn't clear, but this PR goes in an opposite direction: since we know where the problem with compile time and testing is, we make sure that everything else works and is tested as expected, otherwise we might overlook on some things and introduce bugs or other issues, or forget to test something because we don't check the coverage.

@JoshOrndorff
Copy link
Contributor

Okay, I'm coming around to this more and more.

actions-rs has been archived and is no longer maintained.

Damn. I didn't realize that. That is a loss and I hope the community carries on that work somehow. I've found it very useful.

My point wasn't so much that we need to use that specific thing. My point was that I want the CI to be simple, straight-forward, idiomatic, etc. As opposed to custom, complex, hard-to-understand, etc.

We aren't CI experts (at least I'm not) and I don't really want to be one. I just want something simple that tells me if the tests pass. That's what I meant when I was talking about those actions-res examples.

I got the rocks-db dependency removed again over in #156 and I thought maybe that would save us enough disk space to not need this. But it wasn't enough.

@muraca
Copy link
Collaborator Author

muraca commented Dec 13, 2023

We might try to test parachain with the --release flag to see if it takes less space.

Signed-off-by: muraca <mmuraca247@gmail.com>
Signed-off-by: muraca <mmuraca247@gmail.com>
Signed-off-by: muraca <mmuraca247@gmail.com>
Signed-off-by: muraca <mmuraca247@gmail.com>
Signed-off-by: muraca <mmuraca247@gmail.com>
Signed-off-by: muraca <mmuraca247@gmail.com>
Signed-off-by: muraca <mmuraca247@gmail.com>
@muraca muraca mentioned this pull request Dec 14, 2023
@muraca
Copy link
Collaborator Author

muraca commented Dec 14, 2023

superseded by #159

@muraca muraca closed this Dec 14, 2023
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.

2 participants