CTQ status is not reliably reported to Slack#continouous-testing #166

pixelzoom · 2023-03-05T15:59:55Z

CTQ is not showing up reliably in this Slack channel. For example, looking at the CT page, number-suite-common has been failing lint since 3/3/2023 2:52:40 PM (see below) and as of 3/5/2023 8:48 AM is not showing up in Slack#continouous-testing.

If there are multiple CTQ errors, is something considering them all fixes when only 1 of them has been fixed?

Assigning to @zepumph @jonathanolson to investigate, @kathy-phet to prioritize.

pixelzoom · 2023-03-05T16:49:01Z

In Slack#continouous-testing, @kathy-phet said:

@chrisklus - can you please take a quick look at the above on Monday [3/6] and comment on possible issues, I believe you were leading the work to consolidate the slack messaging piece of CT a while back. But let me know if I’m mistaken there.

pixelzoom · 2023-03-12T04:31:06Z

I hit this again today. Buggy code in to the new 'community' repo. It's failing lint in CTQ, reported on the CT webpage, not reported in Slack#continuous-testing.

Raising priority to top, because PhET depends heavily on CT and CTQ. And assigning to @kathy-phet to prioritize.

chrisklus · 2023-03-14T16:02:52Z

@zepumph and I are planning to investigate this afternoon.

zepumph · 2023-03-14T23:07:34Z

@chrisklus and I spent an hour on this just now and only barely got to a point where we could iterate and understand the code effectively. We made many improvements along the way and if this issue should remain a priority. It should not try to be worked into the cracks of this iteration, but should have real time devoted to it (and I'd be very happy to get to work on it!).

We will check back in tomorrow.

zepumph · 2023-04-14T20:54:09Z

Today @chrisklus and I overhauled the quick server to get it ready for parsing these messages better. We have a few leads on where the problems are coming from, but our best guess is that we change the lint output so that it has individual repos as they run, and then an "all results" section. We should just parse that part for CTQ.

* Factor out methods from main loop * Simplify forceTests and waiting logic * Send slack a message upon first run and passing * Isolate individual tests to a single line each

…er formatting, etc), phetsims/aqua#166

zepumph · 2023-04-25T21:05:38Z

I was able to find and fix a fair number of bugs about the lint error parsing, now I'm going to move on to tsc checking which is (on my machine at least) reporting errors like tsc: . (blank). Fun!

1. Support windows using backslashes for file paths. 2. Instead of line.length use a line regex (with row:column + error detection) 3. Fix case missing last filename because we need to recheck when we have a currentFilename and the `line` is the next filename.

…er formatting, etc), #166

…mand, #166

zepumph · 2023-04-25T22:16:06Z

I made good progress. With the help of @marlitas and @samreid the tsc command is not much easier to parse and report on. I pushed these new changes to sparky ctq. We will see if there is any trouble before too long.

zepumph · 2023-04-25T22:23:15Z

As part of this issue I'd like to look into reportErrorStatus, documenting and updating if needed (but mostly trying to understand it).

…ion, #166

zepumph · 2023-04-26T17:18:33Z

Ok. I have completed the work I see for this issue. All changes above are now pushed to production on sparky's ct-quick process. From here I think it would be best for @chrisklus to do the review.

@pixelzoom, please let us know if you run into any more trouble.

chrisklus · 2023-04-28T17:17:18Z

@zepumph and I did much of this review together to get me oriented and then I did some testing on my side and a bit more code review. Things are looking great! I tested all of the cases I could think of, including throwing an Error in code which @zepumph mentioned he had not tried out. It didn't always show up but I think that was just because sometimes the sim doesn't start up for the allotted time in the test case.

@zepumph and I ran into one issue where i didn't have an npm module and so an unexpected lint error triggered an assertion which was a failure farther downstream than we were expecting. I looked into this a little bit and found that the error was not sending an stderr, but I'm not sure why.

The only thing I noticed behavior wise was that a fuzz error reports as new error every loop. I think this may be because the server port is different every time? So it doesn't recognize it as the same error. But, it's good that it's not continuously adding to the number of errors every time, so that when it is fixed, there aren't a bunch of reported pre-existing errors still around. I'm not thinking this is worth fixing since I'm not sure I've ever seen a fuzz error show up on CTQ in the history of it's life, and it would probably be a lot of work.

CTQ additional failure:
simFuzz: Error: Error: Error: Assertion failed: Screen tandems should end with Screen suffix
    at window.assertions.assertFunction (http://localhost:59717/assert/js/assert.js:28:13)
    at new Screen (http://localhost:59717/chipper/dist/js/joist/js/Screen.js:120:17)
    at new LabScreen (http://localhost:59717/chipper/dist/js/natural-selection/js/lab/LabScreen.js:39:5)
    at http://localhost:59717/chipper/dist/js/natural-selection/js/natural-selection-main.js:17:78
    at Namespace.window.phet.joist.launchSimulation (http://localhost:59717/chipper/dist/js/joist/js/simLauncher.js:62:9)
    at http://localhost:59717/chipper/dist/js/joist/js/simLauncher.js:80:27
    at http://localhost:59717/chipper/dist/js/phet-core/js/asyncLoader.js:38:42
    at Array.forEach (<anonymous>)
    at AsyncLoader.proceedIfReady (http://localhost:59717/chipper/dist/js/phet-core/js/asyncLoader.js:38:22)
    at Image.<anonymous> (http://localhost:59717/chipper/dist/js/phet-core/js/asyncLoader.js:51:12)
3 pre-existing errors remain.

[4:28](https://phetsims.slack.com/archives/C03G9D6NY07/p1682634486002929)
CTQ additional failure:
simFuzz: Error: Error: Error: Assertion failed: Screen tandems should end with Screen suffix
    at window.assertions.assertFunction (http://localhost:59733/assert/js/assert.js:28:13)
    at new Screen (http://localhost:59733/chipper/dist/js/joist/js/Screen.js:120:17)
    at new LabScreen (http://localhost:59733/chipper/dist/js/natural-selection/js/lab/LabScreen.js:39:5)
    at http://localhost:59733/chipper/dist/js/natural-selection/js/natural-selection-main.js:17:78
    at Namespace.window.phet.joist.launchSimulation (http://localhost:59733/chipper/dist/js/joist/js/simLauncher.js:62:9)
    at http://localhost:59733/chipper/dist/js/joist/js/simLauncher.js:80:27
    at http://localhost:59733/chipper/dist/js/phet-core/js/asyncLoader.js:38:42
    at Array.forEach (<anonymous>)
    at AsyncLoader.proceedIfReady (http://localhost:59733/chipper/dist/js/phet-core/js/asyncLoader.js:38:22)
    at Image.<anonymous> (http://localhost:59733/chipper/dist/js/phet-core/js/asyncLoader.js:51:12)
3 pre-existing errors remain.

[4:28](https://phetsims.slack.com/archives/C03G9D6NY07/p1682634505594419)
CTQ additional failure:
simFuzz: Error: Error: Error: Assertion failed: Screen tandems should end with Screen suffix
    at window.assertions.assertFunction (http://localhost:59750/assert/js/assert.js:28:13)
    at new Screen (http://localhost:59750/chipper/dist/js/joist/js/Screen.js:120:17)
    at new LabScreen (http://localhost:59750/chipper/dist/js/natural-selection/js/lab/LabScreen.js:39:5)
    at http://localhost:59750/chipper/dist/js/natural-selection/js/natural-selection-main.js:17:78
    at Namespace.window.phet.joist.launchSimulation (http://localhost:59750/chipper/dist/js/joist/js/simLauncher.js:62:9)
    at http://localhost:59750/chipper/dist/js/joist/js/simLauncher.js:80:27
    at http://localhost:59750/chipper/dist/js/phet-core/js/asyncLoader.js:38:42
    at Array.forEach (<anonymous>)
    at AsyncLoader.proceedIfReady (http://localhost:59750/chipper/dist/js/phet-core/js/asyncLoader.js:38:22)
    at Image.<anonymous> (http://localhost:59750/chipper/dist/js/phet-core/js/asyncLoader.js:51:12)
3 pre-existing errors remain.

Back to @zepumph if anything else to do before close.

zepumph · 2023-04-28T18:13:51Z

So still to investigate:

lint.js should provide stderr when facing problems (likely from the composite structure of linting individual repos)
Figure out how to make sure puppeteer fails to CTQ if you have a very upstream error like throw new Error() in main
Update the "comparison" logic for fuzzing errors to ignore /localhost:\d} so that ports don't cause reporting to think there is a new error.

…166

zepumph · 2023-04-28T21:28:50Z

Figure out how to make sure puppeteer fails to CTQ if you have a very upstream error like throw new Error() in main

Yes I'm pretty sure you are right. I also encountered it only sometimes throwing an error because the sim fuzz is only for 1 second when testing.

zepumph · 2023-04-28T21:51:45Z

Alright. With a bit of a weird try catch in lint, we can force stderr from occurring. This means that problems with Line will be handled separately, upstream of trying to parse the lint errors (which would likely result in a false negative).

Everything else here has been handled. Thanks so much @chrisklus for the timely review.

…er formatting, etc), phetsims/aqua#166

pixelzoom added type:bug type:automated-testing labels Mar 5, 2023

pixelzoom assigned jonathanolson, kathy-phet and zepumph Mar 5, 2023

pixelzoom assigned chrisklus and unassigned jonathanolson, kathy-phet and zepumph Mar 5, 2023

pixelzoom assigned kathy-phet Mar 12, 2023

pixelzoom added the priority:1-top label Mar 12, 2023

chrisklus unassigned kathy-phet Mar 14, 2023

zepumph mentioned this issue Mar 14, 2023

Improve CTQ notifier #152

Closed

4 tasks

zepumph added a commit that referenced this issue Mar 14, 2023

added doc and "isTestMode" in spots to simplify testing case, #166

232de8d

zepumph self-assigned this Mar 14, 2023

marlitas added the type:emergent/choice label Mar 17, 2023

zepumph added a commit that referenced this issue Apr 14, 2023

lots of refactoring and updates #166:

dd691ea

* Factor out methods from main loop * Simplify forceTests and waiting logic * Send slack a message upon first run and passing * Isolate individual tests to a single line each

zepumph added a commit to phetsims/chipper that referenced this issue Apr 14, 2023

update about messaging to be used in Quick Server, phetsims/aqua#166

295929e

zepumph added a commit that referenced this issue Apr 14, 2023

trim redundant lint output and only care about "all results", #166

d07935f

jonathanolson added a commit that referenced this issue Apr 24, 2023

quick status tests, see #166

ae3111c

jonathanolson added a commit that referenced this issue Apr 24, 2023

quickNode fix to add .tests, see #166

f814e90

zepumph added a commit to phetsims/chipper that referenced this issue Apr 25, 2023

Further improvements to lint handling and regex (trimming lines, bett…

6da1488

…er formatting, etc), phetsims/aqua#166

zepumph added a commit that referenced this issue Apr 25, 2023

Further improvements to lint handling and regex (trimming lines, bett…

0ec5d63

…er formatting, etc), #166

zepumph added a commit that referenced this issue Apr 25, 2023

update tsc command for easier parsing, plus remove empty lines, #166

abe65f2

zepumph added a commit that referenced this issue Apr 25, 2023

report stderr if a problem occurred from trying to run an execute com…

e505ecb

…mand, #166

zepumph added a commit that referenced this issue Apr 25, 2023

remove transpile as a test with results (just do it), #166

18b858d

zepumph added a commit that referenced this issue Apr 25, 2023

Promote TODO to issue, #166

afab8da

zepumph added a commit that referenced this issue Apr 26, 2023

documentation and refactoring for organization and duplication-reduct…

d305cb9

…ion, #166

zepumph added a commit that referenced this issue Apr 26, 2023

document handleBrokenState and simplify checkForNewErrors, #166

e0cf40d

zepumph added the status:ready-for-review label Apr 26, 2023

zepumph removed their assignment Apr 26, 2023

chrisklus assigned zepumph and unassigned chrisklus Apr 28, 2023

zepumph removed the priority:1-top label Apr 28, 2023

zepumph added a commit that referenced this issue Apr 28, 2023

phetioWrapperDebug=true for testing studio, #166

d12135c

zepumph added a commit that referenced this issue Apr 28, 2023

change random port to always 8080 to test for consistent fuzz errors, #…

b694311

…166

zepumph added a commit that referenced this issue Apr 28, 2023

updates to how to handle errors from stderr, #166

73cb012

zepumph added a commit to phetsims/chipper that referenced this issue Apr 28, 2023

make sure that errors with lint end up on stderr, phetsims/aqua#166

cdea78d

zepumph closed this as completed Apr 28, 2023

samreid pushed a commit to phetsims/perennial that referenced this issue Oct 17, 2024

update about messaging to be used in Quick Server, phetsims/aqua#166

4c2dc16

samreid pushed a commit to phetsims/perennial that referenced this issue Oct 17, 2024

Further improvements to lint handling and regex (trimming lines, bett…

bc36de6

…er formatting, etc), phetsims/aqua#166

samreid pushed a commit to phetsims/perennial that referenced this issue Oct 17, 2024

make sure that errors with lint end up on stderr, phetsims/aqua#166

2b3073e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CTQ status is not reliably reported to Slack#continouous-testing #166

CTQ status is not reliably reported to Slack#continouous-testing #166

pixelzoom commented Mar 5, 2023 •

edited

Loading

pixelzoom commented Mar 5, 2023 •

edited

Loading

pixelzoom commented Mar 12, 2023 •

edited by kathy-phet

Loading

chrisklus commented Mar 14, 2023

zepumph commented Mar 14, 2023

zepumph commented Apr 14, 2023

zepumph commented Apr 25, 2023

zepumph commented Apr 25, 2023

zepumph commented Apr 25, 2023 •

edited

Loading

zepumph commented Apr 26, 2023

chrisklus commented Apr 28, 2023

zepumph commented Apr 28, 2023 •

edited

Loading

zepumph commented Apr 28, 2023

zepumph commented Apr 28, 2023

CTQ status is not reliably reported to Slack#continouous-testing #166

CTQ status is not reliably reported to Slack#continouous-testing #166

Comments

pixelzoom commented Mar 5, 2023 • edited Loading

pixelzoom commented Mar 5, 2023 • edited Loading

pixelzoom commented Mar 12, 2023 • edited by kathy-phet Loading

chrisklus commented Mar 14, 2023

zepumph commented Mar 14, 2023

zepumph commented Apr 14, 2023

zepumph commented Apr 25, 2023

zepumph commented Apr 25, 2023

zepumph commented Apr 25, 2023 • edited Loading

zepumph commented Apr 26, 2023

chrisklus commented Apr 28, 2023

zepumph commented Apr 28, 2023 • edited Loading

zepumph commented Apr 28, 2023

zepumph commented Apr 28, 2023

pixelzoom commented Mar 5, 2023 •

edited

Loading

pixelzoom commented Mar 5, 2023 •

edited

Loading

pixelzoom commented Mar 12, 2023 •

edited by kathy-phet

Loading

zepumph commented Apr 25, 2023 •

edited

Loading

zepumph commented Apr 28, 2023 •

edited

Loading