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

remove assets test #1112

Merged
merged 2 commits into from
Feb 7, 2023
Merged

Conversation

rajatjindal
Copy link
Collaborator

remove assets test as this testcase has been migrated to new e2e tests

pub async fn assets_routing_works(controller: &dyn Controller) {

Signed-off-by: Rajat Jindal rajatjindal83@gmail.com

Signed-off-by: Rajat Jindal <rajatjindal83@gmail.com>
@itowlson
Copy link
Contributor

itowlson commented Feb 6, 2023

I feel there was some value in having "integration lite" tests with no dependencies (or at most Bindle) that could be run locally relatively quickly and easily, though I'm not sure I ever really ended up catching or diagnosing an issue with them, so eh, maybe not. I guess I don't have strong feelings either way. grin

@rajatjindal
Copy link
Collaborator Author

Hi Ivan

It is a fair point. Thank you for bringing it up.

If you are able to run tests using docker compose file (checked-in as part of repo already), will that be a reasonable workaround?

I still need to optimize the docker compose process for local laptop and also for arm, but it generally worked for me. I can also add this to Makefile if that makes it any easier.

What do u think

@itowlson
Copy link
Contributor

itowlson commented Feb 6, 2023

Ah, yep, if I can run specific tests (a la cargo test my_test) in the Docker harness then that should be grand. Thanks!

Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

Assuming @itowlson is 👍 ; LGTM

Copy link
Contributor

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

ah yep sorry :shipit:

Signed-off-by: Rajat Jindal <rajatjindal83@gmail.com>
@rajatjindal
Copy link
Collaborator Author

Ah, yep, if I can run specific tests (a la cargo test my_test) in the Docker harness then that should be grand. Thanks!

I just pushed a change to add entrypoint to docker compose file to demonstrate how to use (or change) that for running specific tests using docker compose.

@rajatjindal rajatjindal merged commit 66c5c6f into fermyon:main Feb 7, 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.

4 participants