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

Execute Release-Tests last on CI #13349

Open
wants to merge 25 commits into
base: Pharo13
Choose a base branch
from

Conversation

jecisc
Copy link
Member

@jecisc jecisc commented Apr 7, 2023

We have a lot of tests in Pharo and it might happen that some of them do not respect the principle of letting the system clean with their tearDown. To help to spot those impacts, I propose to run the releases tests last. Like this, if some bad generated code/or modifications of system entities happens, we might catch it with the verifications of release tests.

I expect this commit to break the tests in the CI but it'll help me find the culprits and fix them.

We have a lot of tests in Pharo and it might happen that some of them do not respect the principle of letting the system clean with their tearDown.
To help to spot those impacts, I propose to run the releases tests last. Like this, if some bad generated code/or modifications of system entities happens, we might catch it with the verifications of release tests.

I expect this commit to break the tests in the CI but it'll help me find the culprits and fix them.
@privat
Copy link
Contributor

privat commented Apr 7, 2023

Nice.

Since tests force the coverage (of) some code, thus performs code transformation because of deprecation, is there a release test that check that the code is pristine/unmodified?

@jecisc
Copy link
Member Author

jecisc commented Apr 7, 2023

There is none yet, but I would love to have that :)

@jecisc
Copy link
Member Author

jecisc commented Apr 7, 2023

But we will need some work to get it because in Pharo 12 it comes with dirty packages, so this test would fail currently

@jecisc
Copy link
Member Author

jecisc commented Apr 7, 2023

More than 10 tests failures because the tests are dirtying the image D:

@jecisc jecisc added the Status: Need more work The issue is nearly ready. Waiting some last bits. label Apr 8, 2023
@jecisc
Copy link
Member Author

jecisc commented Apr 8, 2023

Here is a first fix: pharo-spec/NewTools#504

jecisc added a commit to jecisc/pharo that referenced this pull request Apr 9, 2023
ClassRenameFixTest is way too complicated for what it does.
This aim to simplify it and make it cleaner.

Currently running this test was leaving the system in a dirty state leaving an empty package. (cf. pharo-project#13349)

Now, instead of saving all the changes and trying to revert it, the code generated is all in a specific package that we remove in the tear down.
I removed a double initialization.
And finally I inlined #renameClassUsing: because this method was doing almost everything and the test nothing
@jecisc jecisc reopened this Apr 11, 2023
@jecisc jecisc added Status: Need more work The issue is nearly ready. Waiting some last bits. and removed Status: Need more work The issue is nearly ready. Waiting some last bits. labels Nov 13, 2023
@jecisc jecisc mentioned this pull request Nov 27, 2023
@jecisc jecisc changed the base branch from Pharo12 to Pharo13 April 26, 2024 23:22
@jecisc jecisc closed this May 7, 2024
@jecisc jecisc reopened this May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Need more work The issue is nearly ready. Waiting some last bits.
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

2 participants