Move tests #1001

numero-744 · 2022-12-18T21:15:50Z

TL;DR: gl hf

This PR moves tests to their standard location according to SBT. To do so:

"test" dependencies between the different subprojects were added (see build.sbt)
The things that I had to put common (or I thought I had to) were moved to tester, and its organization in packages should be discussed (eg: sim vs formal)
I could not associate some tests with a specific item / file so for them, they are in integration folder of the appropriate sub-project.

TODO list:

Find and apply a better name than fixes
Check licenses
Squash "only rename" commits
Organize tester/main (suggestions appreciated)

Closes #982

It is just moving items for now, maybe more structure would be better.

Also moves ddr package files to ddr folder

Readon · 2022-12-19T04:51:28Z

So the dependency graph becomes complex though.

numero-744 · 2022-12-19T07:37:59Z

I would say we just add one single rule: in tests, everything can access to everything 😅

Didn't change release (main) dependency graph.

Readon · 2023-01-04T15:42:14Z

core/src/main/scala/spinal/core/Vec.scala

@@ -311,7 +311,7 @@ class VecBitwisePimper[T <: Data with BitwiseOp[T]](pimped : Vec[T]) extends Bit

  private def map2with(f: (T, T) => T)(other: Vec[T]): Vec[T] = {
    if (pimped.length != other.length)
-      SpinalError(s"Cannot apply a bitwize opration on vectors with different size (${pimped.length} vs ${other.length})")
+      SpinalError(s"Cannot apply a bitwize operation on vectors with different sizes (${pimped.length} vs ${other.length})")


Would you please commit this individually?

I've added in this PR a fixup! commit cancelling this change. I'll squash this fixup with the appropriate commit when rebasing before merging this PR (I don't rebase/force push for now else you will hate me ^^').

I've added the reversion of this new commit in a new local branch I'll publish later, when things don't move anymore ^^'

core/src/test/scala/integration/OperatorTester.scala

Readon · 2023-01-04T15:49:12Z

tester/src/test/scala/fixes/Issue963.scala

@@ -1,10 +1,10 @@
-package spinal.tester.scalatest
+package fixes


This package name is a little bit weird.

I initially put issues but then found a PR test which had no issue attached to it, so this is why I have chosen this name: to fix bugs.

Maybe something like bugs, ghbugs, bughistory?

Readon · 2023-01-04T15:51:30Z

tester/src/main/scala/spinal/tester/RepeatabilitySuite.scala

+  def checkOutputHash(gen: => Component): Unit = {
+    checkOutputHashCounter = checkOutputHashCounter + 1
+    var ref = ""
+    for (i <- 0 until 8) {


What's the reason that constant 8 and 50MHz should be fixed?

I have split into a lot of small commits to make it possible to find things back in history (please don't squash).

This part comes from c90ddf5#diff-0733323f702a7bd2f7f1b39fa25465375e98deb1811887c62089e95caecc2b23R466-R467

kleinai · 2023-01-04T21:53:01Z

I have a few concerns. You should keep the refactoring to as little changes as necessary. I understand package names and some imports will change. I also feel we should squash these commits down to avoid creating a large commit log of "refractor moved X" that's annoying to look at. Listing all the files out in fewer commits is fine.

Also if we're moving files from core, lib, and test we need to make sure the licenses are compatible. We probably haven't thought about this in the past but there's a considerable amount of code moving around now.

numero-744 · 2023-01-05T07:58:04Z

I planned to have a semi-linear history for this, to ease reading the graph, but yes we can consider reducing the number of commits.

My concern is that sometimes there were several files testing the same thing; so I have put them together into a single file to match the SBT convention. In this case, mixing all the commits together might be annoying because it makes it harder to find from which file comes this added code chunk. If the blame shows that it comes from a commit with only:

the file modified with adds
and one deleted file

then, the code comes from this deleted file. See #1001 (comment)

To sum up:

history readability (that you mentioned)
code traceability (that I mention)

What do you think about squashing commits which "only rename" files (even if there are some modifications)? So that we reduce the number of commits (maybe a lot?) while keeping good code traceability for when files are merged together.

Note: for now I'm working fast-forward only, I'll rebase and squash things when we agree on the code we get.

I think I have only moved files from tester/scalatest and I don't know what are the rules for this project, should check this. As we'll publish tester (the structure has to be defined, for now I just have put everything in the same folder) we should clearly define that it is the same license as lib IMO.

Readon · 2023-01-05T15:59:16Z

I have a few concerns. You should keep the refactoring to as little changes as necessary. I understand package names and some imports will change. I also feel we should squash these commits down to avoid creating a large commit log of "refractor moved X" that's annoying to look at. Listing all the files out in fewer commits is fine.

Also if we're moving files from core, lib, and test we need to make sure the licenses are compatible. We probably haven't thought about this in the past but there's a considerable amount of code moving around now.

I am also think that one PR simply move the tester into right directory is enough.
More patchs can be commit by other PR, it's really too big to review now.

Readon · 2023-01-17T17:15:04Z

When would this be finished?^^
The existing test is really take too much time to finish.

numero-744 · 2023-01-17T17:34:37Z

@Readon thanks for the ping, it is nice to know that what I've been working on will be useful 😃

I think I won't be able to spend significant time on this PR until early Feb but:

I will need feedback for the following points (among the points of the TODO list in the PR description) so this PR can progress through discussion before I'm back on it:

Find a better package name than fixes (see Move tests #1001 (comment))
Check licenses (maybe delay that until later review?) (see Move tests #1001 (comment))
Define an organization of packages for tester/main

When I'll be back on this PR I'll be able to:

Squash "only rename" commits to reduce commits number as suggested in Move tests #1001 (comment)
Apply suggestions according to the feedback (especially change fixes package name if needed and organize tester/main)
Rebase to fix merge conflicts + move more stuff if needed + re-squash if needed
Force-push that for review
Remove draft status

Readon · 2023-01-18T01:07:38Z

@Readon thanks for the ping, it is nice to know that what I've been working on will be useful 😃

I think I won't be able to spend significant time on this PR until early Feb but:

I will need feedback for the following points (among the points of the TODO list in the PR description) so this PR can progress through discussion before I'm back on it:

Find a better package name than fixes (see Move tests #1001 (comment))

Check licenses (maybe delay that until later review?) (see Move tests #1001 (comment))

Define an organization of packages for tester/main

When I'll be back on this PR I'll be able to:

Squash "only rename" commits to reduce commits number as suggested in Move tests #1001 (comment)

Apply suggestions according to the feedback (especially change fixes package name if needed and organize tester/main)

Rebase to fix merge conflicts + move more stuff if needed + re-squash if needed

Force-push that for review

Remove draft status

Maybe the first step is just move those test into their corresponding directory to speed up the CI.
^^

Dolu1990 · 2023-01-18T12:41:21Z

thanks for the ping, it is nice to know that what I've been working on will be useful

Sure :D

The main thing to be carefull about, is to not let such massive PR too long in flight, as the more the time pass, the more conflict may popup.

fixes

could be regression ? or something else, as you feel it ^^

andreasWallner · 2023-02-05T13:10:43Z

Maybe another ping ;-)
To throw in another possible name: spinal.issues (since we are currently referring to issues in those?)
Long term those might make more sense as TestIssueXxx tests with theirs respective modules I think, that would be the most likely place to look for them?

Dolu1990 · 2023-02-06T10:53:42Z

@andreasWallner
Yes why not ^^

Readon · 2023-11-02T09:40:11Z

I think this have been done, so can we close this? @numero-744

Dolu1990 · 2023-11-02T10:46:48Z

It isn't fully completted, but better to do it from upstream i think

Readon · 2023-11-03T15:20:40Z

It isn't fully completted, but better to do it from upstream i think

What is still missing currently？

Dolu1990 · 2023-11-06T10:45:09Z

What is still missing currently？

https://github.com/SpinalHDL/SpinalHDL/tree/dev/tester/src/test/scala/spinal/tester/scalatest is still full of things :)

Readon · 2023-11-06T11:32:44Z

What is still missing currently？

https://github.com/SpinalHDL/SpinalHDL/tree/dev/tester/src/test/scala/spinal/tester/scalatest is still full of things :)

ahh, that is only remained for the balance for running time of ci test.

andreasWallner · 2023-11-06T12:56:54Z

I think that we should not choose a folder structure solely to split test execution.
If we need to split there are a bunch of ways (tags, running tests for specific packages, etc.)

numero-744 added 30 commits December 18, 2022 17:04

refactor: move CocotbBase to tester/main

9b280b4

refactor: rename AhbLite3CrossbarFactory.scala

a6cd359

refactor: move AhbLite3CrossbarTester

2a6c0ca

refactor: move AhbLite3OnChipRamTester

0adc1ca

refactor: move Axi4AdapterTester

720dc9e

refactor: move Axi4CrossbarTester

7cf5d28

refactor: move Apb3I2cSlaveTester

3339f43

refactor: move Apb3SpiMasterCtrlTester

a7c793e

refactor: move Apb3SpiSlaveCtrlTester

6843ca5

refactor: move Axi4CrossbarTester2

1d8507f

refactor: move Axi4SharedSdramCtrlTesterCocotbBoot

429476e

refactor: move Axi4SlaveFactoryTester

dc089d7

refactor: move Axi4StreamSimpleWidthAdapterTester

9538a6b

refactor: move Axi4StreamTester

ba06688

refactor: move Axi4StreamWidthAdapterTester

75cc1d6

refactor: move AxiLite4SlaveFactoryTester

da712fd

build: add test dependencies to core

ebc44ab

refactor: mv more tester/test to tester/main

33f742f

It is just moving items for now, maybe more structure would be better.

refactor: move BlackboxTester

b9c3057

refactor: move LatchTester

8c79c61

refactor: move BundleTester

4248134

refactor: move WhenTester

2ad7330

refactor: move UartTester

c023d75

refactor: move SpinalTesterGhdlBase to tester/main

966fa1b

refactor: move StreamTester

e051d01

refactor: move StreamTester2

577facb

refactor: move MemTester

bcb6854

refactor: move InOutTester

a3ca3b0

refactor: move GrayCounterTester

0dde973

refactor: move UsbDeviceCtrlTester

e5d80af

numero-744 added 4 commits December 18, 2022 17:04

refactor: move SpinalSimSpiXdrMaster

038dae7

Also moves ddr package files to ddr folder

refactor: move SpinalSimDmaSgTester

461e597

refactor: move SpinalSimUsbHostTester

5e97414

refactor: move SdramXdrSdrSpinalSim

8b14b41

numero-744 marked this pull request as draft December 18, 2022 21:16

numero-744 mentioned this pull request Jan 4, 2023

feat(lib): Axi4 to AxiLite4 conversions #1002

Merged

2 tasks

Readon reviewed Jan 4, 2023

View reviewed changes

fixup! refactor: move SpinalSimLibTester

dbfc6bb

This was referenced Jan 6, 2023

feat(regif): apb4 support and busif improvment #976

Merged

feat: SpinalTestBench #968

Open

numero-744 mentioned this pull request Jan 13, 2023

ci: cache management + code organization #1017

Merged

2 tasks

Readon mentioned this pull request May 2, 2023

An experimental PR to use docker image github provided as a CI environment. #1099

Merged

Readon mentioned this pull request Sep 3, 2023

move tests to speed up ci. #1192

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move tests #1001

Move tests #1001

numero-744 commented Dec 18, 2022 •

edited

Readon commented Dec 19, 2022

numero-744 commented Dec 19, 2022

Readon Jan 4, 2023

numero-744 Jan 4, 2023

Readon Jan 4, 2023

numero-744 Jan 4, 2023

Readon Jan 4, 2023

numero-744 Jan 4, 2023

kleinai commented Jan 4, 2023

numero-744 commented Jan 5, 2023

Readon commented Jan 5, 2023

Readon commented Jan 17, 2023

numero-744 commented Jan 17, 2023 •

edited

Readon commented Jan 18, 2023

Dolu1990 commented Jan 18, 2023

andreasWallner commented Feb 5, 2023

Dolu1990 commented Feb 6, 2023

Readon commented Nov 2, 2023

Dolu1990 commented Nov 2, 2023

Readon commented Nov 3, 2023

Dolu1990 commented Nov 6, 2023

Readon commented Nov 6, 2023

andreasWallner commented Nov 6, 2023

		@@ -1,10 +1,10 @@
		package spinal.tester.scalatest
		package fixes

Move tests #1001

Are you sure you want to change the base?

Move tests #1001

Conversation

numero-744 commented Dec 18, 2022 • edited

Readon commented Dec 19, 2022

numero-744 commented Dec 19, 2022

Readon Jan 4, 2023

Choose a reason for hiding this comment

numero-744 Jan 4, 2023

Choose a reason for hiding this comment

Readon Jan 4, 2023

Choose a reason for hiding this comment

numero-744 Jan 4, 2023

Choose a reason for hiding this comment

Readon Jan 4, 2023

Choose a reason for hiding this comment

numero-744 Jan 4, 2023

Choose a reason for hiding this comment

kleinai commented Jan 4, 2023

numero-744 commented Jan 5, 2023

Readon commented Jan 5, 2023

Readon commented Jan 17, 2023

numero-744 commented Jan 17, 2023 • edited

Readon commented Jan 18, 2023

Dolu1990 commented Jan 18, 2023

andreasWallner commented Feb 5, 2023

Dolu1990 commented Feb 6, 2023

Readon commented Nov 2, 2023

Dolu1990 commented Nov 2, 2023

Readon commented Nov 3, 2023

Dolu1990 commented Nov 6, 2023

Readon commented Nov 6, 2023

andreasWallner commented Nov 6, 2023

numero-744 commented Dec 18, 2022 •

edited

numero-744 commented Jan 17, 2023 •

edited