Refactor testApi.mjs #1055
Refactor testApi.mjs #1055
Conversation
zaneenders
force-pushed
the
zane-new-server-test
branch
from
afc9aca to
2e06758
Compare
| formula: fpcore, sample: p_context | ||
| }), async_huh); | ||
| assertIdAndPath(analyze_json); | ||
| checkFelids(analyze_json, ['command', 'points', 'job', 'path']); |
There was a problem hiding this comment.
| checkFelids(analyze_json, ['command', 'points', 'job', 'path']); | |
| checkFeilds(analyze_json, ['command', 'points', 'job', 'path']); |
| const check_missing_job = await fetchAndCheckRSPHeaders(`/check-status/42069`, { method: 'GET' }, CHECK_CORS) | ||
| assert.equal(check_missing_job.status, 202); | ||
|
|
||
| // MARK: UP |
There was a problem hiding this comment.
What does this // MARK: UP do?
| const analyze_json = await getJSONFor('analyze', 'POST', JSON.stringify({ | ||
| formula: fpcore, sample: p_context | ||
| }), async_huh); | ||
| assertIdAndPath(analyze_json); |
There was a problem hiding this comment.
I think you call this every time you call getJSONFor, except for the mathjs translator. Move into getJSONFor?
| const exacts1 = await callExacts(FPCoreFormula2, eval_sample, true); | ||
| const exacts2 = await callExacts(FPCoreFormula2, eval_sample, false); |
There was a problem hiding this comment.
I think every test is repeated twice, once in async mode and once not. Maybe build that into getJSONFor instead of having to do it manually?
|
|
||
| // MARK: Sample | ||
| async function callSample(fpcore, async_huh) { | ||
| var sample_json = await getJSONFor('sample', 'POST', JSON.stringify({ formula: fpcore, seed: 5 }), async_huh); |
There was a problem hiding this comment.
As far as I can tell, getJSONFor is always called with a method of POST and the body is always JSON. Can we move that into getJSONFor?
| const timelineRSP = await fetchAndCheckRSPHeaders(`/timeline/${job_iD}`, { method: 'GET' }, CHECK_CORS); | ||
| assert.equal(timelineRSP.status, 201); |
There was a problem hiding this comment.
Are we fetching the timeline here just to check that it was a valid job id? This is a weird test.
| const improveResponse = await fetch(makeEndpoint(`/improve?formula=${encodeURIComponent(FPCoreFormula2)}`), { method: 'GET' }) | ||
| assert.equal(improveResponse.status, 200); | ||
| let redirect = improveResponse.url.split("/"); | ||
| const job_iD = redirect[3].split(".")[0]; |
There was a problem hiding this comment.
Why the super weird capitalization in job_iD?
|
|
||
| // improve-start endpoint | ||
| const URIencodedBody = "formula=" + encodeURIComponent(FPCoreFormula) | ||
| const startResponse = await fetch(makeEndpoint("/api/start/improve"), { |
There was a problem hiding this comment.
Why do all the other tests use a callXXX function but this one is fetch(makeEndpoint(...))? Why not at least wrap the fetch stuff so we wouldn't need to manually write makeEndpoint? And maybe the method, headers, etc? Maybe call encodeURIComponent if that's important?
| while (checkStatus.status != 201 && counter < cap) { | ||
| counter += 1 | ||
| checkStatus = await fetch(makeEndpoint(improveResultPath), { method: 'GET' }) | ||
| await new Promise(r => setTimeout(r, 100)); // ms | ||
| } |
There was a problem hiding this comment.
I guess it's fine but isn't this a for loop with an early exit? Also I notice that even when the status is successful you still wait for 100ms before exiting.
| (response/full 201 | ||
| (response/full 200 |
There was a problem hiding this comment.
Look, I guess whatever, but in this case I think a 201 does seem more appropriate? Since it doesn't include the body?
This is essentially a new
testApi.mjsfile refactored to compose functions likesimulateOdysseyCall.