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

feat: only instantiate test resources once in Pharo #662

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

theseion
Copy link
Collaborator

@theseion theseion commented Sep 17, 2024

Fixes #661

Note: also includes a change to the project loading code. The Moose images already contain SmalltalkCI, which prevented the changes in this PR from being picked up. We now force load SmalltalkCI in Pharo and GToolkit builds.

@Rinzwind
Copy link
Contributor

Looks OK. A slight improvement that could be made is to make #testResourcesSetupOnce assert that the counters are nil before the send of #runSpec: so as to make the change to the counters clear in the test. The exclusion of SCIPharoTestSuiteNestingTestCase1 and SCIPharoTestSuiteNestingTestCase2 could perhaps be handled by adding them to the #exclude in ‘.smalltalk.ston’ instead of through the reflective #isAbstract implementation.

@theseion
Copy link
Collaborator Author

Looks OK. A slight improvement that could be made is to make #testResourcesSetupOnce assert that the counters are nil before the send of #runSpec: so as to make the change to the counters clear in the test.

Good idea.

The exclusion of SCIPharoTestSuiteNestingTestCase1 and SCIPharoTestSuiteNestingTestCase2 could perhaps be handled by adding them to the #exclude in ‘.smalltalk.ston’ instead of through the reflective #isAbstract implementation.

I chose #isAbstract on purpose, to prevent these tests from being available for running in the image as well. They would fail and that would be difficult to understand.

@theseion theseion force-pushed the setup-test-resources-only-once branch from af83952 to 51ea314 Compare September 19, 2024 07:16
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 resources are set up multiple times
2 participants