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

Fix test and add new ones to close 878 #971

Merged
merged 2 commits into from Nov 25, 2022

Conversation

marimeireles
Copy link
Member

Fixed the tests that were xfail or failing + added a new test to make @antocuni happy.
Intends to close #878

Copy link
Contributor

@antocuni antocuni left a comment

Choose a reason for hiding this comment

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

Fixed the tests that were xfail or failing + added a new test to make @antocuni happy. Intends to close #878

😂😂😂
It's not to make me happy, it's to make the codebase better ;).

Ah right, I didn't remember that we had already a test marked as xfail: good! But I also like the new test, good 💪

@marimeireles
Copy link
Member Author

marimeireles commented Nov 22, 2022

I need to lint it 😅
Alright, of course is to make the code base better but I have to be honest and say that 50% of the reason why I added it was because I thought you'd like it.

@madhur-tandon
Copy link
Member

Is this waiting on the CI to pass? If yes, I think #978 fixes it.
Maybe @marimeireles we can wait for that to be merged and then you can rebase on main for this PR.

@marimeireles
Copy link
Member Author

Thanks @madhur-tandon will try now! ;)

@marimeireles
Copy link
Member Author

Ah no, I guess it's something else.
Will address it now.

@marimeireles
Copy link
Member Author

Okay. I have no idea why this is failing.
I don't think it's related to what I'm doing here, it's prbb smth else.
Specially cause it was green before and now it's red.
So... maybe #978 didn't properly fix?

@FabioRosado
Copy link
Contributor

I'm not sure why but I had to rebase main into my other branch for the tests to pick up the changes

@marimeireles
Copy link
Member Author

Interesting, alright =)

@marimeireles
Copy link
Member Author

Ah, well cause it's running from the previous yamls
Makes sense 😅

@marimeireles marimeireles merged commit 30e31a8 into pyscript:main Nov 25, 2022
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.

display() sends the output to the wrong tag
4 participants