Skip to content

Commit

Permalink
refactor: use neoforge framework
Browse files Browse the repository at this point in the history
Stop using Guava RateLimiter as the gametests run ticks faster, and the RateLimiter uses the system
clock so RS wouldn't keep up with the faster tick rate.
  • Loading branch information
raoulvdberge committed May 25, 2024
1 parent 33d83f0 commit a283a03
Show file tree
Hide file tree
Showing 13 changed files with 217 additions and 50 deletions.
55 changes: 37 additions & 18 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ Valid examples are:

## Translations

If you want to contribute to the translations of this project, please use [Crowdin](https://crowdin.com/project/refined-storage-2).
If you want to contribute to the translations of this project, please
use [Crowdin](https://crowdin.com/project/refined-storage-2).

## Versioning

Expand Down Expand Up @@ -126,8 +127,25 @@ at all times.
They ensure that there are no regressions, act as general documentation for the codebase,
and ensure that the project can evolve over time.

To avoid brittle tests, tests need to validate behavior. A test cannot rely on the internal code structure, so most
mocking should be avoided.
To avoid brittle tests, tests need to validate behavior. A test cannot rely on the internal code structure, so most use
of mocking should be avoided.

### Unit testing

Tests in the API modules are regular unit tests. Don't see a "unit" here as a code unit, but as a unit of behavior.

> Don't isolate code by using mocking, even in a unit test.
These tests don't rely on, nor know about, Minecraft.

Additionally, tests in the `refinedstorage2-network` module use the `refinedstorage2-network-test` JUnit plugin to easily set up networks for testing.

### Integration testing

To test the entire chain from Minecraft to the API modules, we use integration tests. These tests are located in the
test source set of the `refinedstorage2-platform-forge` module.

We write these integration tests as Minecraft gametests using the NeoForge testing framework.

### Test coverage

Expand Down Expand Up @@ -202,6 +220,7 @@ The build workflow takes care of the following:

- Running a Gradle build, running our tests in the process and generating an aggregated code coverage report for the API
modules.
- Running Minecraft gametests.
- Analyzing the code on SonarQube.
> Because of
> [limitations with SonarQube](https://portal.productboard.com/sonarsource/1-sonarcloud/c/50-sonarcloud-analyzes-external-pull-request),
Expand Down Expand Up @@ -249,18 +268,18 @@ Refined Storage 2 is split up into various modules.
Most modules aren't dependent on Minecraft or a mod loader. Only modules that start
with `refinedstorage2-platform-*` have dependencies on Minecraft.

| Name | Use in addons | Description |
|-----------------------------------|---------------|---------------------------------------------------------------------------------------|
| `refinedstorage2-core-api` | ✔️ | Contains some utility classes and enums. |
| `refinedstorage2-grid-api` | ✔️ | Contains Grid related functionality. |
| `refinedstorage2-network-api` | ✔️ | Contains storage network related functionality. |
| `refinedstorage2-network` || Contains implementations of `refinedstorage2-network-api`. |
| `refinedstorage2-network-test` | ✔️ | JUnit extension which helps with setting up a network and a network node for testing. |
| `refinedstorage2-query-parser` | ✔️ | A query parser, contains a lexer and parser. Only used for Grid query parsing. |
| `refinedstorage2-resource-api` | ✔️ | Contains API for handling resources. |
| `refinedstorage2-storage-api` | ✔️ | Contains storage related functionality. |
| `refinedstorage2-platform-api` | ✔️ | Implements the various Refined Storage API modules for use in Minecraft. |
| `refinedstorage2-platform-test` | ✔️ | This module is used in platform tests for various Minecraft related helpers. |
| `refinedstorage2-platform-fabric` || The platform module for Fabric. This module contains Fabric specific code. |
| `refinedstorage2-platform-forge` || The platform module for Forge. This module contains Forge specific code. |
| `refinedstorage2-platform-common` || Common mod code. Most gameplay code is in here. |
| Name | Use in addons | Description |
|-----------------------------------|---------------|----------------------------------------------------------------------------------------------------|
| `refinedstorage2-core-api` | ✔️ | Contains some utility classes and enums. |
| `refinedstorage2-grid-api` | ✔️ | Contains Grid related functionality. |
| `refinedstorage2-network-api` | ✔️ | Contains storage network related functionality. |
| `refinedstorage2-network` || Contains implementations of `refinedstorage2-network-api`. |
| `refinedstorage2-network-test` | ✔️ | JUnit extension which helps with setting up a network and a network node for testing. |
| `refinedstorage2-query-parser` | ✔️ | A query parser, contains a lexer and parser. Only used for Grid query parsing. |
| `refinedstorage2-resource-api` | ✔️ | Contains API for handling resources. |
| `refinedstorage2-storage-api` | ✔️ | Contains storage related functionality. |
| `refinedstorage2-platform-api` | ✔️ | Implements the various Refined Storage API modules for use in Minecraft. |
| `refinedstorage2-platform-test` | ✔️ | This module is used in platform tests for various Minecraft related helpers. |
| `refinedstorage2-platform-fabric` || The platform module for Fabric. This module contains Fabric specific code. |
| `refinedstorage2-platform-forge` || The platform module for Forge. This module contains Forge specific code and the integration tests. |
| `refinedstorage2-platform-common` || Common mod code. Most gameplay code is in here. |
1 change: 1 addition & 0 deletions config/checkstyle/checkstyle-suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@
<!-- Shadow target contains underscore -->
<suppress checks="MemberName" files="ModelBakerImplMixin.java"/>
<suppress checks="HideUtilityClassConstructor" files="GridClearPacket.java"/>
<suppress checks="HideUtilityClassConstructor" files="TestMod.java"/>
</suppressions>
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public ConstructorBlockEntity(final BlockPos pos, final BlockState state) {
}

@Override
protected void setFilters(final List<ResourceKey> filters) {
public void setFilters(final List<ResourceKey> filters) {
this.tasks.clear();
this.tasks.addAll(filters.stream().map(TaskImpl::new).toList());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ protected void setTaskExecutor(final TaskExecutor<ExporterNetworkNode.TaskContex
}

@Override
protected void setFilters(final List<ResourceKey> filters) {
public void setFilters(final List<ResourceKey> filters) {
mainNode.setFilters(filters);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ public Direction extractDirection(@Nullable final BlockState state) {
return getDirectionType().extractDirection(direction);
}

public BlockState rotated(final T direction) {
return defaultBlockState().setValue(getDirectionType().getProperty(), direction);
}

public static boolean doesBlockStateChangeWarrantNetworkNodeUpdate(final BlockState oldBlockState,
final BlockState newBlockState) {
if (!(newBlockState.getBlock() instanceof AbstractDirectionalBlock<?> newDirectionalBlock)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,5 +82,5 @@ public void writeScreenOpeningData(final ServerPlayer player, final FriendlyByte

protected abstract void setTaskExecutor(TaskExecutor<C> taskExecutor);

protected abstract void setFilters(List<ResourceKey> filters);
public abstract void setFilters(List<ResourceKey> filters);
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import java.util.ArrayList;
import java.util.List;

import com.google.common.util.concurrent.RateLimiter;
import net.minecraft.core.BlockPos;
import net.minecraft.core.NonNullList;
import net.minecraft.nbt.CompoundTag;
Expand All @@ -33,7 +32,8 @@ public abstract class AbstractUpgradeableNetworkNodeContainerBlockEntity<T exten
private static final String TAG_UPGRADES = "u";

protected final UpgradeContainer upgradeContainer;
private RateLimiter rateLimiter = createRateLimiter(0);
private int workTickRate = 20;
private int workTicks;

protected AbstractUpgradeableNetworkNodeContainerBlockEntity(
final BlockEntityType<?> type,
Expand All @@ -52,7 +52,7 @@ protected AbstractUpgradeableNetworkNodeContainerBlockEntity(

@Override
public final void doWork() {
if (rateLimiter.tryAcquire()) {
if (workTicks++ % workTickRate == 0) {
super.doWork();
postDoWork();
}
Expand Down Expand Up @@ -105,16 +105,12 @@ public void load(final CompoundTag tag) {
private void configureAccordingToUpgrades() {
LOGGER.debug("Reconfiguring {} for upgrades", getBlockPos());
final int amountOfSpeedUpgrades = upgradeContainer.getAmount(Items.INSTANCE.getSpeedUpgrade());
this.rateLimiter = createRateLimiter(amountOfSpeedUpgrades);
this.workTickRate = (amountOfSpeedUpgrades + 1) * 20;
this.setEnergyUsage(upgradeContainer.getEnergyUsage());
}

protected abstract void setEnergyUsage(long upgradeEnergyUsage);

private static RateLimiter createRateLimiter(final int amountOfSpeedUpgrades) {
return RateLimiter.create((double) amountOfSpeedUpgrades + 1);
}

@Override
public NonNullList<ItemStack> getDrops() {
final NonNullList<ItemStack> drops = NonNullList.create();
Expand Down
14 changes: 14 additions & 0 deletions refinedstorage2-platform-forge/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,18 @@ dependencies {
compileOnly "top.theillusivec4.curios:curios-neoforge:7.1.0+1.20.4:api"
}

sourceSets {
test {
minecraft {
modIdentifier = 'rstest'
}
}
}

runs {
gameTestServer {
systemProperty 'neoforge.enabledGameTestNamespaces', 'refinedstorage2_tests'
}
}

enablePublishing()
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package com.refinedmods.refinedstorage2.platform.forge;

import com.refinedmods.refinedstorage2.api.resource.ResourceAmount;
import com.refinedmods.refinedstorage2.platform.common.constructordestructor.ConstructorBlockEntity;
import com.refinedmods.refinedstorage2.platform.common.storage.ItemStorageType;

import java.util.List;

import net.minecraft.core.Direction;
import net.minecraft.gametest.framework.GameTest;
import net.minecraft.world.level.block.Blocks;
import net.neoforged.testframework.annotation.TestHolder;
import net.neoforged.testframework.gametest.EmptyTemplate;
import net.neoforged.testframework.gametest.ExtendedGameTestHelper;

import static com.refinedmods.refinedstorage2.platform.forge.TestMod.DIRT;
import static com.refinedmods.refinedstorage2.platform.forge.TestMod.RSBLOCKS;
import static com.refinedmods.refinedstorage2.platform.forge.TestMod.itemIsInserted;
import static com.refinedmods.refinedstorage2.platform.forge.TestMod.storageMustContainExactly;
import static net.minecraft.core.BlockPos.ZERO;

public final class ConstructorTest {
private ConstructorTest() {
}

@GameTest
@EmptyTemplate(value = "5x5x5", floor = true)
@TestHolder(description = "Tests whether the Constructor can place a block")
public static void shouldPlaceBlock(final ExtendedGameTestHelper helper) {
// Arrange
helper.setBlock(ZERO.above(), RSBLOCKS.getCreativeController().getDefault());
helper.setBlock(ZERO.above().above(), RSBLOCKS.getConstructor().getDefault().rotated(Direction.EAST));
helper.setBlock(
ZERO.above().above().above(),
RSBLOCKS.getItemStorageBlock(ItemStorageType.Variant.ONE_K)
);

final var seq = helper.startSequence();
seq.thenWaitUntil(itemIsInserted(helper, ZERO.above().above(), DIRT, 10));

// Act
helper.requireBlockEntity(ZERO.above().above(), ConstructorBlockEntity.class).setFilters(List.of(DIRT));

// Assert
seq.thenWaitUntil(() -> helper.assertBlockPresent(Blocks.DIRT, ZERO.above().above().east()))
.thenWaitUntil(storageMustContainExactly(helper, ZERO.above().above(), new ResourceAmount(DIRT, 9)))
.thenSucceed();
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package com.refinedmods.refinedstorage2.platform.forge;

import com.refinedmods.refinedstorage2.api.core.Action;
import com.refinedmods.refinedstorage2.api.network.Network;
import com.refinedmods.refinedstorage2.api.network.node.NetworkNode;
import com.refinedmods.refinedstorage2.api.network.storage.StorageNetworkComponent;
import com.refinedmods.refinedstorage2.api.resource.ResourceAmount;
import com.refinedmods.refinedstorage2.api.storage.EmptyActor;
import com.refinedmods.refinedstorage2.platform.api.support.network.AbstractNetworkNodeContainerBlockEntity;
import com.refinedmods.refinedstorage2.platform.common.content.Blocks;
import com.refinedmods.refinedstorage2.platform.common.support.resource.ItemResource;
import com.refinedmods.refinedstorage2.platform.common.util.IdentifierUtil;

import java.util.Arrays;
import javax.annotation.Nullable;

import net.minecraft.core.BlockPos;
import net.minecraft.gametest.framework.GameTestAssertException;
import net.minecraft.resources.ResourceLocation;
import net.minecraft.world.item.ItemStack;
import net.minecraft.world.item.Items;
import net.neoforged.bus.api.IEventBus;
import net.neoforged.fml.ModLoadingContext;
import net.neoforged.fml.common.Mod;
import net.neoforged.testframework.conf.FrameworkConfiguration;
import net.neoforged.testframework.gametest.ExtendedGameTestHelper;

@Mod(TestMod.ID)
public class TestMod {
public static final String ID = IdentifierUtil.MOD_ID + "_tests";

public static final ItemResource DIRT = ItemResource.ofItemStack(new ItemStack(Items.DIRT));
public static final Blocks RSBLOCKS = Blocks.INSTANCE;

public TestMod(final IEventBus eventBus) {
final var framework = FrameworkConfiguration.builder(new ResourceLocation(ID, "tests"))
.build()
.create();
framework.init(eventBus, ModLoadingContext.get().getActiveContainer());
}

@Nullable
public static Network getNetwork(final ExtendedGameTestHelper helper, final BlockPos pos) {
try {
final var be = helper.requireBlockEntity(pos, AbstractNetworkNodeContainerBlockEntity.class);
final var field = AbstractNetworkNodeContainerBlockEntity.class.getDeclaredField("mainNode");
field.setAccessible(true);
final NetworkNode mainNode = (NetworkNode) field.get(be);
return mainNode.getNetwork();
} catch (IllegalAccessException | IllegalArgumentException | NoSuchFieldException | SecurityException e) {
throw new RuntimeException(e);
}
}

public static Runnable itemIsInserted(final ExtendedGameTestHelper helper,
final BlockPos networkPos,
final ItemResource resource,
final long amount) {
return () -> {
final Network network = getNetwork(helper, networkPos);
helper.assertTrue(network != null && network.getComponent(StorageNetworkComponent.class)
.insert(resource, amount, Action.EXECUTE, EmptyActor.INSTANCE) == amount,
"Item couldn't be inserted"
);
};
}

public static Runnable storageMustContainExactly(final ExtendedGameTestHelper helper,
final BlockPos networkPos,
final ResourceAmount... expected) {
return () -> {
final Network network = getNetwork(helper, networkPos);
helper.assertTrue(network != null, "Network is not found");
if (network == null) {
return;
}
final StorageNetworkComponent storage = network.getComponent(StorageNetworkComponent.class);
for (final ResourceAmount expectedItem : expected) {
final boolean contains = storage.getAll()
.stream()
.anyMatch(inStorage -> inStorage.getResource().equals(expectedItem.getResource())
&& inStorage.getAmount() == expectedItem.getAmount());
if (!contains) {
throw new GameTestAssertException("Missing from storage: " + expectedItem);
}
}
for (final ResourceAmount inStorage : storage.getAll()) {
final boolean wasExpected = Arrays.stream(expected).anyMatch(
expectedItem -> expectedItem.getResource().equals(inStorage.getResource())
&& expectedItem.getAmount() == inStorage.getAmount()
);
if (!wasExpected) {
throw new GameTestAssertException("Unexpected in storage: " + inStorage);
}
}
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
modLoader = "javafml"
loaderVersion = "[2,)"
license = "MIT"
[[mods]]
modId = "refinedstorage2"
[[mods]]
modId = "refinedstorage2_tests"
Binary file not shown.

0 comments on commit a283a03

Please sign in to comment.