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

Review of failed tests logic and Integration Tests #5

Merged
merged 11 commits into from
Nov 17, 2023

Conversation

lleites
Copy link
Contributor

@lleites lleites commented Nov 13, 2023

FIxes #1

Now tests fail either by an unhandled Error or by a failed assertion from the Mojo standard lib testing module.
By just using the standard lib assertions, it is possible to collect all the items, even if one of them fails in between.

I Also added Integration Tests as a GitHub Workflow, for that to work you need to setup the MODULAR_AUTH GitHub secret with the value of the Modular key you can find at https://developer.modular.com/download, check the curl command.

The documentation is not updated yet, I am waiting for us to discuss the implementation to do it.

@guidorice guidorice self-requested a review November 15, 2023 00:04
Copy link
Owner

@guidorice guidorice left a comment

Choose a reason for hiding this comment

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

@lleites I like what you have done overall. Nice improvements! I am on board with this.
I made a few inline requests for changes, and I also added my MODULAR_AUTH secret to this repo. 👍🏽

Hopefully we can provide an upgrade path for anyone happening to use 0.1.0 already.
I tried installing your fork on my repo https://github.com/guidorice/geo-features and almost all my tests failed with list index out of range.
Not sure, but maybe it is because self.spec["stdout"][-1] is indexing into an empty list. See my comments in MojoTestItem. I think it must be related. Not sure what is causing it exactly.

If you want to take an attempt at updating the Readme, that would be welcome as well.

example/tests/util.mojo Outdated Show resolved Hide resolved
example/tests/util.mojo Outdated Show resolved Hide resolved
example/tests/util.mojo Show resolved Hide resolved
pytest_mojo/plugin.py Show resolved Hide resolved
pytest_mojo/plugin.py Outdated Show resolved Hide resolved
@lleites
Copy link
Contributor Author

lleites commented Nov 17, 2023

Thank you for your comments, let me address them one by one.
1 - I am unable to reproduce the error in your repository, it works well for me.

Steps I did and still got all green tests:

pip uninstall pytest-mojo 
pip install git+https://github.com/lleites/mojo-pytest.git 

Can you please try to uninstall and install the plugin again? Do you have mojo available in your path?
First, please be sure to have a green setup with the current mojo-pytest version

Full log attached, see how I move from version 0.1.0 to 0.1.1 of the plugin
reinstallation.log

@lleites
Copy link
Contributor Author

lleites commented Nov 17, 2023

README.md updated with new conventions

@guidorice
Copy link
Owner

@lleites, yes it is working for me. I tried all the modes of passing and failing tests. Not sure what I did wrong before, probably messed up my conda environment. :)

@guidorice guidorice self-requested a review November 17, 2023 23:49
Copy link
Owner

@guidorice guidorice left a comment

Choose a reason for hiding this comment

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

👍🏽 thanks

@guidorice guidorice merged commit e1d885b into guidorice:main Nov 17, 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.

Test items are not collected after Error is raised (within a test_* file)
2 participants