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

⭐ Automated tests for all syntax and changes. #6158

Open
Moderocky opened this issue Oct 31, 2023 · 5 comments
Open

⭐ Automated tests for all syntax and changes. #6158

Moderocky opened this issue Oct 31, 2023 · 5 comments
Labels
good first issue An issue that would be good for a first-time contributor to make a PR for

Comments

@Moderocky
Copy link
Member

Moderocky commented Oct 31, 2023

In the future, we want to set a standard of every contribution providing its own tests.

Why?

These automated tests can be run before a build, meaning we have a guarantee that anything in the test files will perform as expected.

On top of that, the tests are run for each pull request before being merged, so if a contribution accidentally breaks or changes the behaviour of other syntax in an unexpected way we will know about this before release.

For example, if I open a pull request that changes parsing behaviour and it accidentally breaks the parsing of an obscure expression, the tests will fail and catch my mistake.

Where do we start?

To begin with, I would like all existing syntax to have tests (where applicable), even if they are just very basic.
Test coverage is incredibly poor currently.

Type Coverage Percent
Expressions 34/284 12%
Conditions 11/85 13%
Effects 10/79 13%
Sections 3/4 75%

The existing test coverage is displayed below, for anybody who would like to help out by filling in some of these missing gaps.

Expressions

  • ExprItemWithCustomModelData
  • ExprArrowsStuck
  • ExprLevelProgress
  • ExprVectorDotProduct
  • ExprMaxMinecartSpeed
  • ExprParse
  • ExprCursorSlot
  • ExprLocationFromVector
  • ExprPlayerViewDistance
  • ExprValueWithin
  • ExprSeaPickles
  • ExprAttacker
  • ExprPermissions
  • ExprVehicle
  • ExprNoDamageTicks
  • ExprTimePlayed
  • ExprSpawnReason
  • ExprInventoryAction
  • ExprValue
  • ExprShooter
  • ExprSpawnerType
  • ExprColoured
  • ExprHash
  • ExprFallDistance
  • ExprLength
  • ExprSourceBlock
  • ExprSaturation
  • ExprTypeOf
  • ExprClientViewDistance
  • ExprBookAuthor
  • ExprIndexOf
  • ExprPortal
  • ExprDefaultValue
  • ExprLocation
  • ExprLoot
  • ExprEnchantmentOffer
  • ExprEnchantItem
  • LitAt
  • ExprRandomUUID
  • ExprCommandInfo
  • ExprFireTicks
  • ExprShuffledList
  • ExprAmount
  • ExprTimeSince
  • ExprVectorRandom
  • ExprCustomModelData
  • ExprTime
  • ExprMaxHealth
  • ExprEnderChest
  • ExprRemainingAir
  • ExprVersion
  • ExprSpecialNumber
  • ExprExplosionYield
  • ExprLevel
  • ExprWorldEnvironment
  • ExprUnixDate
  • ExprEnchantingExpCost
  • ExprAffectedEntities
  • ExprTargetedBlock
  • ExprVectorFromYawAndPitch
  • ExprMetadata
  • ExprBlock
  • ExprPlain
  • ExprExhaustion
  • ExprDamagedItem
  • ExprEnchantments
  • ExprCmdCooldownInfo
  • ExprHighestSolidBlock
  • ExprLocationOf
  • ExprYawPitch
  • ExprInventory
  • ExprHealReason
  • ExprFertilizedBlocks
  • ExprMiddleOfLocation
  • ExprAbsorbedBlocks
  • ExprHumidity
  • ExprLocationVectorOffset
  • ExprSpeed
  • ExprSeed
  • ExprFoodLevel
  • ExprNow
  • ExprMaxFreezeTicks
  • ExprSortedList
  • ExprSets
  • ExprUnixTicks
  • ExprParseError
  • ExprArmorSlot
  • ExprAge
  • ExprLastDamageCause
  • ExprTimeState
  • ExprEnchantmentOfferCost
  • ExprNumbers
  • ExprHatchingType
  • ExprVectorNormalize
  • ExprWorlds
  • ExprLoopValue
  • ExprVersionString
  • ExprExperience
  • ExprVectorFromXYZ
  • ExprLightLevel
  • ExprChunk
  • ExprPing
  • ExprJoinSplit
  • ExprNamed
  • ExprMaxDurability
  • ExprCoordinate
  • ExprPlayerlistHeaderFooter
  • ExprEventExpression
  • ExprAI
  • ExprLastColor
  • ExprOps
  • ExprExplodedBlocks
  • ExprItems
  • ExprTimes
  • ExprInventorySlot
  • ExprLastAttacker
  • ExprSlotIndex
  • ExprTotalExperience
  • ExprLeashHolder
  • ExprOfflinePlayers
  • ExprEyeLocation
  • ExprLanguage
  • ExprFurnaceSlot
  • ExprGlowing
  • ExprTamer
  • LitPi
  • ExprUUID
  • ExprAppliedEnchantments
  • ExprCommandSender
  • ExprHiddenPlayers
  • ExprGameMode
  • ExprAttackCooldown
  • ExprExplosiveYield
  • ExprChatRecipients
  • ExprScripts
  • ExprNumberOfCharacters
  • ExprIndices
  • ExprBreakSpeed
  • ExprFilter
  • ExprVectorSquaredLength
  • ExprMe
  • ExprRandomNumber
  • ExprPotionEffectTier
  • ExprColorOf
  • ExprMoonPhase
  • ExprDifference
  • ExprMendingRepairAmount
  • ExprLastLoadedServerIcon
  • ExprSubstring
  • ExprPassenger
  • ExprVectorOfLocation
  • ExprPlayerWeather
  • ExprBlockSphere
  • ExprBlockData
  • ExprBurnCookTime
  • ExprRespawnLocation
  • ExprVectorSpherical
  • ExprExplosionBlockYield
  • ExprSeaLevel
  • ExprAnvilText
  • ExprGravity
  • ExprLore
  • ExprDirection
  • ExprEnchantmentBonus
  • ExprProtocolVersion
  • ExprPushedBlocks
  • ExprLastLoginTime
  • ExprProjectileBounceState
  • ExprVectorXYZ
  • ExprMinecartDerailedFlyingVelocity
  • ExprRedstoneBlockPower
  • ExprDropsOfBlock
  • ExprTemperature
  • ExprArrowKnockbackStrength
  • ExprInventoryInfo
  • ExprStringCase
  • ExprScript
  • ExprHotbarSlot
  • ExprHoverList
  • ExprHealAmount
  • ExprTarget
  • ExprSpectatorTarget
  • ExprUnbreakable
  • ExprDifficulty
  • ExprWorldFromName
  • ExprReversedList
  • ExprEntityTamer
  • ExprEntity
  • ExprFinalDamage
  • ExprAllBannedEntries
  • ExprNearestEntity
  • ExprVelocity
  • ExprSkull
  • ExprArgument
  • ExprHotbarButton
  • ExprRawString
  • ExprBookTitle
  • ExprWeather
  • ExprSignText
  • ExprMOTD
  • ExprVectorAngleBetween
  • ExprCommand
  • ExprIP
  • ExprItem
  • ExprFreezeTicks
  • ExprAllCommands
  • ExprDateAgoLater
  • ExprEventCancelled
  • ExprHatchingNumber
  • ExprServerIcon
  • ExprPotionEffect
  • ExprLastSpawnedEntity
  • ExprHanging
  • ExprItemsIn
  • ExprGameRule
  • ExprPlayerProtocolVersion
  • ExprLocationAt
  • ExprOnlinePlayersCount
  • ExprDamageCause
  • ExprVectorLength
  • ExprTPS
  • ExprMaxPlayers
  • ExprHealth
  • ExprDrops
  • ExprTeleportCause
  • ExprElement
  • ExprFormatDate
  • ExprItemAmount
  • ExprAmountOfItems
  • ExprLastResourcePackResponse
  • ExprEnchantmentLevel
  • ExprLastDamage
  • ExprCompassTarget
  • ExprProjectileCriticalState
  • ExprBlockHardness
  • ExprEntityAttribute
  • ExprBed
  • ExprTernary
  • ExprDamage
  • ExprEntities
  • LitConsole
  • ExprChatFormat
  • ExprMessage
  • ExprBlocks
  • ExprTool
  • ExprPlugins
  • ExprAltitude
  • ExprVectorArithmetic
  • ExprOpenedInventory
  • ExprWhitelist
  • ExprRound
  • ExprFacing
  • ExprVectorBetweenLocations
  • ExprAlphabetList
  • ExprDurability
  • LitNewLine
  • ExprHostname
  • ExprChestInventory
  • ExprBookPages
  • ExprDistance
  • ExprVectorCylindrical
  • ExprItemFrameSlot
  • ExprWorld
  • ExprFireworkEffect
  • ExprPickupDelay
  • ExprItemWithLore
  • ExprVectorCrossProduct
  • ExprSpawn
  • ExprMaxStack
  • ExprScoreboardTags
  • ExprCreeperMaxFuseTicks
  • ExprFlightMode
  • ExprCharges
  • ExprAttacked
  • ExprArrowPierceLevel
  • ExprClicked
  • ExprRawName
  • ExprEgg
  • ExprXOf
  • ExprRandom
  • ExprName
  • ExprPotionEffects
  • ExprBiome
  • ExprGlidingState

Conditions

  • CondIsVectorNormalized
  • CondIsRiptiding
  • CondIsBanned
  • CondIsPoisoned
  • CondIsFlying
  • CondIsOccluding
  • CondItemInHand
  • CondIsSolid
  • CondCompare
  • CondIsBlockRedstonePowered
  • CondIsSet
  • CondIsInteractable
  • CondMinecraftVersion
  • CondIsInvisible
  • CondLeashed
  • CondIsEmpty
  • CondIsBlock
  • CondIncendiary
  • CondMatches
  • CondPvP
  • CondIsPassable
  • CondIsSwimming
  • CondCancelled
  • CondIsSprinting
  • CondIsFlammable
  • CondIsOp
  • CondScriptLoaded
  • CondEntityIsInLiquid
  • CondIsValid
  • CondIsUnbreakable
  • CondIsOnGround
  • CondIsWhitelisted
  • CondResourcePack
  • CondWithinRadius
  • CondCanSee
  • CondIsTameable
  • CondIsSkriptCommand
  • CondContains
  • CondAlphanumeric
  • CondIsStackable
  • CondIsGliding
  • CondHasClientWeather
  • CondHasPotion
  • CondHasScoreboardTag
  • CondPlayedBefore
  • CondIsPreferredTool
  • CondHasResourcePack
  • CondRespawnLocation
  • CondIsSlimeChunk
  • CondIsInfinite
  • CondIsSilent
  • CondStartsEndsWith
  • CondIsEdible
  • CondIsFrozen
  • CondAnchorWorks
  • CondAI
  • CondIsFuel
  • CondChance
  • CondEntityIsWet
  • CondHasCustomModelData
  • CondIsSleeping
  • CondIsBurning
  • CondIsPluginEnabled
  • CondIsOfType
  • CondHasMetadata
  • CondIsRiding
  • CondDamageCause
  • CondIsSneaking
  • CondIsEnchanted
  • CondIsOnline
  • CondDate
  • CondCanHold
  • CondIsBlocking
  • CondIsInvulnerable
  • CondPermission
  • CondCanFly
  • CondIsWearing
  • CondProjectileCanBounce
  • CondIsCharged
  • CondWillHatch
  • CondIgnitionProcess
  • CondIsTransparent
  • CondIsWithin
  • CondIsLoaded
  • CondIsAlive

Effects

  • EffPush
  • EffPathfind
  • EffActionBar
  • EffBan
  • EffColorItems
  • EffPlayerInfoVisibility
  • EffOp
  • EffPlayerVisibility
  • IndeterminateDelay
  • EffMakeEggHatch
  • EffExceptionDebug
  • EffShoot
  • EffMessage
  • EffRespawn
  • EffHealth
  • EffInvisible
  • EffCancelDrops
  • EffOpenInventory
  • EffKnockback
  • EffLeash
  • EffCommand
  • EffReplace
  • EffMakeSay
  • EffBroadcast
  • Delay
  • EffKill
  • EffEnchant
  • EffVectorRotateAroundAnother
  • EffInvulnerability
  • EffStopServer
  • EffExplodeCreeper
  • EffReturn
  • EffVisualEffect
  • EffLog
  • EffForceAttack
  • EffExit
  • EffMakeFly
  • EffExplosion
  • EffSilence
  • EffPlaySound
  • EffPotion
  • EffDoIf
  • EffContinue
  • EffToggle
  • EffTree
  • EffSendTitle
  • EffFeed
  • EffKeepInventory
  • EffKick
  • EffIncendiary
  • EffCancelCooldown
  • EffPoison
  • EffCancelEvent
  • EffLoadServerIcon
  • EffSendBlockChange
  • EffTeleport
  • EffDrop
  • EffLook
  • EffChargeCreeper
  • EffFireworkLaunch
  • EffOpenBook
  • EffSendResourcePack
  • EffChange
  • EffIgnite
  • EffSuppressWarnings
  • EffHidePlayerFromServerList
  • EffShear
  • EffConnect
  • EffResetTitle
  • EffEquip
  • EffSwingHand
  • EffLightning
  • EffPvP
  • EffToggleFlight
  • EffVectorRotateXYZ
  • EffVehicle
  • EffBreakNaturally
  • EffStopSound
  • EffScriptFile

Sections

  • SecLoop
  • SecWhile
  • SecConditional
  • EffSecSpawn

What can we test?

We can test almost anything that can be done on a server without players. This means anything involving items, maths, blocks, most entities, world operations and various and sundry bits and pieces.

We can't (comprehensively) test things involving players, things that have to be tested over a server restart or require something external to be changed. We also can't test things like performance or capacity very easily. These tests need to run automatically on any computer building Skript.

How can I do this?

In practice, this is fairly simple. If my pull request adds or changes the functionality of syntax, I just need to create (or update) the corresponding Skript test file.

A lot of pull requests won't need any extra tests. You'll only need to add/change a test file if you're adding/changing new functionality.
That said, if you think you can improve the existing test for a syntax element you are welcome to.

For example, if I change how ExprDurability works, I need to make sure that the ExprDurability.sk file is updated to test my changes.
image
image

(If you add a new syntax you will need to create a new test file for it.)

@Moderocky Moderocky added the good first issue An issue that would be good for a first-time contributor to make a PR for label Oct 31, 2023
@Moderocky Moderocky pinned this issue Oct 31, 2023
@hbbaker
Copy link

hbbaker commented Oct 31, 2023

Hi! I'd like to help add tests for existing functions. I'm new to opensource and I'd like to ask a few questions regarding how these tests should be implemented. Thanks!

@sovdeeth
Copy link
Member

Hi! I'd like to help add tests for existing functions. I'm new to opensource and I'd like to ask a few questions regarding how these tests should be implemented. Thanks!

See https://github.com/SkriptLang/Skript/blob/master/src/test/skript/README.md

@Moderocky
Copy link
Member Author

Hi! I'd like to help add tests for existing functions. I'm new to opensource and I'd like to ask a few questions regarding how these tests should be implemented. Thanks!

Sure! It's fairly simple but if you'd like somebody to walk you through the process please contact one of us in our Discord server :)

@APickledWalrus
Copy link
Member

@Moderocky I think it would be a good idea to establish stronger naming guidelines for tests. What do you think about requiring tests to start with (or just be) the name of the expression. For example, the name of the test for ExprBlockData would just be ExprBlockData. Some files have multiple tests, so in those cases we could use "ExprBlockData - " to differentiate the test names. We may also want to consider whether having multiple tests in one file is practical compared to just "sectioning" tests using comments.

@Moderocky
Copy link
Member Author

For example, the name of the test for ExprBlockData would just be ExprBlockData.

The base test is currently named after the syntax class (like ExprBlockData -> block data)

Some files have multiple tests, so in those cases we could use "ExprBlockData - " to differentiate the test names.

These are usually named <class name> (<thing that requires its own test>) so something like block data (legacy) or block data (1.18+) for example.

I think this is fine since when the test fails that makes it fairly clear what's not working.

What I'm more concerned about is the assertion messages - most testers have taken care to make all of the messages unique (so you can work out which one failed from the message) but ideally I'd like the testing system to be able to determine the line that was the cause of the failure so we don't have to rely on that.

@AyhamAl-Ali AyhamAl-Ali changed the title Automated tests for all syntax and changes. ⭐ Automated tests for all syntax and changes. Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue An issue that would be good for a first-time contributor to make a PR for
Projects
None yet
Development

No branches or pull requests

4 participants